Re: [PATCH] HttpHdrSc c++ refactoring

>> Please find attached a new iteration of the patch for review.
>
>
>> -        temp = xstrndup (item, initiallen + 1);
>> -
>> -        if (!((target = strrchr(temp, ';')) && !strchr (target, '"') && *(target + 1) != '\0'))
>> +        if (!((target = memrchr(item, ';')) && !strchr (target, '"') && *(target + 1) != '\0'))
>
> I shortened the memchr() call in the new line for clarity -- it is not
> relevant for this specific comment. Look at the stuff after "&&". The
> old code was calling strchr() for "target" pointing to a zero-terminated
> "temp" string. The new code calls it for target pointing to a
> non-terminated "item". AFAICT, this means that we may find '"' beyond
> the current item (at best). A bug?

I've reverted the change. While it may improve performance, it's not a
critical code-path and it'll be moot once StringNg hits anyways.

>> +        case SC_MAX_AGE:
>> +             int ma;
>
> IIRC, some compilers complain about a local variable not confined
> "enough" to a single switch case scope (in case the if statements bypass
> the "break" command and the compiler cannot reliably detect that?). I
> tried to quickly find any examples of "bare" case-local variables in
> Squid but failed.
>
> Consider surrounding the whole case with {}.

Done.

New iteration attached, please review it.

Thanks

-- 
    /kinkie