Re: delay_pools.c is buggy in 2.3.STABLE3 :-(

david@luyer.net wrote:

> The initial value in a for loop is not checked against the guard.

Eh?

In C for(a;b;c) ... is equivalent to

a;
while(b) {
   ...
   c;
}

The loop condition is always verified prior to entering the loop block.

This is also true in all languages I know borrowing the for syntax from
C...

> Also remember i and j are unsigned 8-bit entities so 255+1=0.

Not in the code I have..

static void
delayUpdateClass3(class3DelayPool * class3, delaySpecSet * rates, int
incr)
{
    int individual_restore_bytes, network_restore_bytes;
    int mpos;
    unsigned int i, j;
    char individual_255_used;

This is both in Squid-2.3.STABLE3 and in the current Squid-2.4
development sources (only available via CVS or online on the web site).

for (i = 255; i < 255; ++i) will
a) Never enter the loop block at all
b) Even if it did, it will never loop, since 255+1 is larger than 255.

The original loops will loop from 0 to 254, or to 255 if the individual
255 is used. This is of course unless the break (or return) statement
inside the loops ends it prematurely.

I do not claim to know the details of the delay pool structures, but I
persist in saying that your proposed patch does not work from simple C
structure rules.

If you could explain the problem in more detail I am quite sure we could
find a suitable patch if required. I think I understand most of these
loops except for the line reading
        if (i != 255 && class3->network_map[i] == 255)
            return;

But I presume it is some kind of shortcut.

Could you please explain this 255 magic in the maps and map indexes?

I suspect the 255_used functionality is broken in both class 2 and 3,
but it might actually work for class 3 under certain specific conditions
(the whole C or B net used).

If it is what I think it is (255 used as "end of map" marker) then I
propose the code to be changed to have a map length counter instead of
this odd magic.

/Henrik
Received on Mon Jun 12 2000 - 17:30:31 MDT