On Mon, 20 Oct 2003 15:54:58 -0400 (EDT)
Norman Tuttle <[EMAIL PROTECTED]> wrote:
Sorry for jumping in so late, but was out of the net for few weeks...
> This patch fixed the following errors in the cookie code in Flood:
> (1) Original code only handled a single "set-cookie" header for a
> given HTTP message. There can be more than one.
> (2) Original code did not handle a replacement for a cookie value
> already in the "database". The fix searches through and avoids these
> duplicates by replacing.
>
> Please examine the attached diff and apply the changes to the file
> flood_round_robin.c to resolve these issues.
I didn't have time to review your code (will do later this day), but
this patch is a horrible mess. First of all -- it is a reverse patch.
You code is prefixed with minus ('-') rather that plus ('+') which means
this patch can remove your code, not add it. You probably reversed file
order when diffing. While patch has -R option to work around such
situations, it's really difficult to read your patch, to figure out what
exactly you are trying to change. Before we commit your changes, we must
exactly understand the purpose of this patch.
Another issue with your patch is that it has a lot of style
corrections (e.g. space removals). That also makes the patch difficult
to read, since I had to browse through a dozen of style corrections to
get to the code (plus since your patch is reversed, I though that you're
just adding unnecesary spaces). If you have found some style violations,
then prepare separate patch and post it here. You'll score extra points
from our own style purist (Hello Justin ;)
The last thing that doesn't look too good -- comments. You don't need
that large comment block with date before your code. It'll get included
in commit message, so there's no point in cluttering the code with this.
Also comments like this:
break; /* Stop cookie search now! Drop current attempt! */
...don't mean anything usefull. The meaning of break statement is clear
to most C programmers, so your comment only makes line overloaded with
text.
So just go easy on us, and prepare the patch again, this time with
readability in mind. I can promise to review your patch, since cookies
in flood are my special area of interest. Other patches that you posted
recently might have to wait a bit. Justin seems extremally busy (we
aren't able to release flood 1.1 for about a month!) and I need to
take a closer look at them. Needless to say -- those patches also need
to be tweaked as they are suffering from the same problems described
above.
--
regards,
Jacek Prucia
