Re: [PATCH] default constructor for HttpParser

On 03/31/2011 07:07 AM, Amos Jeffries wrote:
>>> 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.
>>
>
> This patch should fix that.
>
> It adds a reset() function to perform the default constructor
> initialization. That can also be shared by the other init code.

Thank you for fixing this.

Consider s/reset/clear/ to match standard terminology. STL also uses
reset(), but to set a new value, not to bring the object into its
default state.

> + /// Set this parser back to an empty state.

s/empty/default/ as parser is not a container and cannot be "empty".

>
> -
> void

Avoid whitespace change.

Cheers,

Alex.
Received on Thu Mar 31 2011 - 15:32:52 MDT