On 10/30/2011 01:17 PM, Kinkie wrote:
> 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?
> + 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 {}.
HTH,
Alex.
