winmm: Sign-compare warning fix (Resend)

Michael Karcher wine at mkarcher.dialup.fu-berlin.de
Fri Dec 12 10:23:44 CST 2008


Am Freitag, den 12.12.2008, 09:07 -0700 schrieb James Mckenzie:
> >  unsigned int i;
> >
> >  ...
> >
> >  for (i = sizeof(foo) / sizeof(foo[0]) - 1; ~i; --i)
> >
> >Tested? No.
> >Readable? Don't ask me... :-)
> I understood it, but is not the goal to avoid the use of C++ constructs in WINE code?
There is no C++ construct in that code. Still i-- would do the same and
is more idiomatic C.

> The real question is why convert a for loop to a while loop?
After the last iteration, the --i (or i--) overflows. In a for loop, the
step statement (the third statement in the loop head) is always executed
before the test (the second statement in the loop head), so there is no
way to prevent the overflow. While this is not a problem per se, as
overflow of unsigned variables is well-defined, testing for "overflow
just happended" tends to be more confusing to the readers than testing
for "is at the end".

If you use a while loop instead, you can arrange the test to be executed
*before* the step is performed, so you have to abort on "i == 0 just
finished" insted of "i == ~0U is next", the former being easier to
understand.

> My guess is to avoid a possible empty or zero length foo.
Even in for loops, the tests is executed before the first iteration, so
it would work in the for loop as well.

> Of course, there should be a check before this for an empty or misformed
> foo[0] avoiding a divide by zero error.
Sorry for the strong language: Either you need to take a second look at
the code we talk about or a second look at an medium-level C textbook.
sizeof is a built-in operator that returns the amount of memory storage
the object foo[0] directly occupies. If foo[0] is a pointer, it is the
size of a pointer, not the size of the allocated memory foo[0] might
point to. As there are no objects of size zero in C or C++,
sizeof(foo[0]) can *never* be zero.

The idiom used here "sizeof(foo) / sizeof(foo[0])" yields the size of an
array "foo". It breaks *horribly*[1] if foo is not an array, but a
pointer. If foo denotes an array, sizeof foo returns the size of the
whole array, which is the size per element multiplied by the number of
elements. The size of a single element is calculated by
sizeof(foo[anything]), anything being zero by convention.

Regards,
  Michael Karcher

1: breaks horribly means in that case: Returns nonsense (if foo[0] is a
structure, usually zero), no compile-time error, no obvious error in
that line (one needs the context to find it, and most people just assume
foo to be an array reading that line).




More information about the wine-devel mailing list