Re: [PATCH] default constructor for HttpParser

From: Amos Jeffries <squid3_at_treenet.co.nz>
Date: Fri, 01 Apr 2011 11:45:32 +1300

On 01/04/11 04:32, Alex Rousskov wrote:
> 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.
>

Okay, all fixed and applied.

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE9 or 3.1.11
   Beta testers wanted for 3.2.0.5
Received on Thu Mar 31 2011 - 22:45:38 MDT

This archive was generated by hypermail 2.2.0 : Fri Apr 01 2011 - 12:00:05 MDT