Re: Style question: comparison between signed and unsigned?

Theodore Y. Ts'o (tytso@MIT.EDU)
Wed, 24 Sep 1997 13:08:19 -0400


Date: Wed, 24 Sep 1997 09:37:34 +0100 (GMT/BST)
From: Mike Jagdis <mike@roan.co.uk>

In my experience it means that programmers start to figure C is
so dumb they have to explicitly cast *everything* - which not
only makes it impossible to read their code but ensures neither
the compiler nor the human has the faintest idea whether the
programmer *really* intended a type conversion or if there is
a bug lurking. Not good.

My favorite example of this was from a version of a very well known
piece of security software, by a famous software company that was
purchased by an even larger computer company in the past year or so.

One early version of the software had a piece of code that looked
something like this:

struct key {
unsigned int length;
char *data;
}

unsigned char get_random_byte();

init_random_key(struct key *key)
{
int i;

for (i=0; i < key->length; i++)
key->data = (char) get_random_byte;
}

Now ---- trick question ---- what's wrong with the above code?

The result was the public key was initialized to constant value, and the
fact that a cast was used completely hid the problem from the compiler.
As you might imagine, this had a pretty horrific impact on the security
of the program! The way it was caught was purely by luck --- a
developer was looking at the actual value of the key while debugging a
completely unrelated problem, and thought it odd that every single byte
of the key was identical....

Moral of the story? Casts are evil, and should be avoided whenever
possible. If you must use a cast, ***think*** before throwing it in.
A cast bypasses all of the compiler's type warnings, and in some cases
leave you with some very subtle bugs.

- Ted