Re: Style question: comparison between signed and unsigned?

Linus Torvalds (torvalds@transmeta.com)
Mon, 22 Sep 1997 16:50:22 -0700 (PDT)


On Mon, 22 Sep 1997, Dean Gaudet wrote:
>
> On 22 Sep 1997, Linus Torvalds wrote:
>
> > int i = read(socket, buffer, sizeof(buffer));
>
> What if sizeof(int) != sizeof(size_t)?

The above is completely ok, doesn't generate a warning, and isn't what I
worry about.

sizeof() returns a size_t, which is unsigned, but that's also exactly what
read() wants as the argument.

And read cannot return any value outside an "int", because it's
constrained by the size of the read (plus the special value "-1"). So the
statement

int i = read(socket, buffer, sizeof(buffer));

is not only the common way to do this, it's also the _correct_ way to do
this.

Now, the PROBLEM is somewhere else (and is a problem that is not dependent
on the size, but purely on the signedness of the types - so even if "i"
had been ssize_t the same problem would happen): the part of the code
snippet that you edited out. This was:

/* check for errors *?
if (i == -1)
return i;

/* check that we got a valid packet */
if (i < sizeof(struct pkthdr))
return SHORT_PACKET;

Now, the PROBLEM is that if gcc warns about signed/unsigned comparisons
(which some versions of gcc do), gcc will totally needlessly (and in my
opinion incorrectly) warn about the second test (ie "i < sizeof(struct
pkthdr)"). Because "i" is signed, but "sizeof" is unsigned.

"i" _has_ to be signed, because of the magic meaning of a negative return
value. This is something very deeply embedded into C - nitpickers can
claim that "read()" is not strictly part of the C language, but the point
is that this is common practice and is used by quite a large number of C
interfaces.

So to get rid of that _spurious_ warning, I'd have to change the test into
something like:

if (i < (int) sizeof(struct pkthdr))
return SHORT_PACKET;

Quite frankly, anybody who claims that the extra cast is a good thing
should be shot on the spot - the extra cast is an abomination and has _no_
redeeming features except to shut up a warning from a compiler that thinks
it knows better than the programmer.

Anybody who thinks that their compiler is smarter than they are probably
disagrees with me. But type-casts are the _single_ most dangerous thing
you can do in C, and a compiler that _encourages_ you to add spurious type
casting for no gain is a bad compiler.

Linus