[haiku-development] Re: CID 8883 : Stray semi-column

  • From: Alex Wilson <yourpalal2@xxxxxxxxx>
  • To: haiku-development@xxxxxxxxxxxxx
  • Date: Wed, 23 Nov 2011 23:06:44 -0700

>> In :
>>  /haiku/trunk/src/add-ons/accelerants/nvidia/engine/nv_acc_dma.c
>>
>> Line 680 and below :
>>
>>                        for (cnt = 0; (tmp && !(tmp & 0x00000001)); tmp >>= 
>> 1, cnt++);
>>                        {
>>                                ACCW(NV4X_WHAT2, cnt);
>>                        }
>>
>>
>> What do you guys think?
>>
>
> Hmmm, that semicolon will be the body of the for loop and the loop
> will do nothing.
> This loop will be followed by the block, executed once.  Does this
> even compile? It should not since cnt shouldnt be in scope outside of
> the loop.

Actually, if you look, you will see that cnt is not a loop variable,
it is only initialized in the loop. It looks like the cnt var is
counting the number of times the loop executes. So this code would
indeed compile. As for the behaviour, you're right, the loop would run
so many times, but the body of the loop would be empty. The {} around
what is supposed to be the loop body would just create an extra level
of local scope, and then destroy it after running the statement inside
once.

As for what the loop is supposed to do, it's a bit hard to say. Each
time through, tmp is divided by two, until the point comes that it is
an odd number (Unless tmp = 0, in which case the loop aborts before
the first run). Each run through,   ACCW(NV4X_WHAT2, cnt) is called,
with the number of divisions made so far, eg. 0, 1, 2, 3...

With variable/macro names like ACCW and NV4X_WHAT2, it's hard to say
much more than that.. (CCW = counter-clockwise? ACCW = accumulate wide
(register)?, ACCW = ACC WHAT? WHAT2 = ????)

Depending on what this code is actually supposed to do, the current
behaviour might be the intended one, although if that's the case, it
should be written like this:

for (cnt = 0; (tmp && !(tmp & 0x00000001)); tmp >>= 1, cnt++)
    ;
ACCW(NV4X_WHAT2, cnt);

Given the {}, I think the author likely intended the call to ACCW to
be made several times. Then again, if everything works nicely right
now, maybe it should be left as is. On the other hand, are we sure
everything really does work nicely right now? Maybe Rudolf can swoop
in and help out?

--Alex

Other related posts: