Re: [PATCH] HttpHdrSc c++ refactoring

>> +/**
>> + * look for the last occurrence of a character in a c-string with a set maximum length
>> + */
>> +SQUIDCEXTERN const char *strnrchr(const char *s, size_t slen, char c);
>
> Please fix the strnrchr() description. To match the implementation, the
> description should say that we scan from the beginning and stop at the
> end of the c-string or n-th character, whichever comes first (and
> s/slen/n/).

Done (although s/slen/count/ to match the

> I would also recommend checking whether your strnrchr() implementation
> matches that of GNU API. Their documentation is even worse:
>  http://gnugeneration.com/mirrors/kernel-api/r2320.html

Done.

>
>
>> -        if (!sctusable || sctusable->content.size() == 0)
>> +        if (!sctusable || sctusable->hasContent())
>
> Sorry if I asked you about this already, but is the reversal of the
> second condition above intentional?

No, it wasn't.
Thanks for spotting that, and sorry for missing it if you asked already :(

>>  class HttpHdrSc
>>  {
>>
>>  public:
>> +    bool parse(const String *str);
>> +    ~HttpHdrSc();
>> +    HttpHdrSc(const HttpHdrSc &);
>
> Please move the parse() declaration below constructors/destructors.

Done.

>> +    HttpHdrSc() {};
>
> Extra semicolon.

Removed.

>>>> >> +    String Content() const { return content; }
>>> >
>>> > I would s/content/content_/ instead, especially since content data
>>> > member is private. Capitalization should be used for static methods.
>>
>> Ok.
>
> If that "Ok" meant "will do later", then please note that you have not
> done that.

Gah! Sorry.. Now done.
New version attached.

-- 
    /kinkie