Re: /bzr/squid3/trunk/ r11314: Cleanup: make clientParseRequest() a member of ConnStateData

On 03/25/2011 08:03 PM, Amos Jeffries wrote:
> ------------------------------------------------------------
> revno: 11314
> committer: Amos Jeffries <squid3_at_treenet.co.nz>
> branch nick: trunk
> timestamp: Sat 2011-03-26 15:03:49 +1300
> message:
> Cleanup: make clientParseRequest() a member of ConnStateData
>
> This allows the HttpParser to also become a member field and persistent
> across all requests on the connection instead of newely allocated on
> the stack for every read cycle.
> That in turn allows the parser to retain state for efficient 'trickle'
> parsing across multiple read cycles.
>
> For now the old behaviour of reset on every read is retained in order to
> prevent this shuffling from causing behaviour changes. That negates most
> of the actual performance gains (for now).

> class HttpParser
> {
> public:
> + HttpParser() {};

This creates a parser in an invalid/random state. We should never do
that. The old code was also broken, but the broken class was not used as
a member and did not have any constructors so it was more-or-less clear
that it is just an old C structure. The new code makes it look like a
good, usable class, but it is not.

Please either remove the default HttpParser constructor or call
HttpParserInit from the default constructor. In both cases, I think you
can use a zero-length NULL or "" buffer to give the parser a valid
initial state.

There is also an extra semicolon after {}.

Thank you,

Alex.
Received on Tue Mar 29 2011 - 16:44:09 MDT