Re: Strange code in ide_cdrom_register

Peter Chubb (peter@chubb.wattle.id.au)
Thu, 30 May 2002 19:05:48 +1000


>>>>> "Michael" == Michael Dunsky <michael.dunsky@p4all.de> writes:

Michael> Hi! Peter Chubb wrote:
PeterC> Hi, This code snippet in ide_cdrom_register() seems really
PeterC> strange...

>> *(int *)&devinfo->speed = CDROM_STATE_FLAGS
>> (drive)->current_speed; *(int *)&devinfo->capacity = nslots;

PeterC> devinfo-> speed and devinfo->capacity are both ints. So the casts are
PeterC> just a disaster waiting to happen, if the types of capacity or
PeterC> speed ever change?

Michael> Just take a quick look in drivers/ide/ide-cd.h: values
Michael> "nslots" and "current_speed" are of type "byte", so we need
Michael> to cast to store them (like that) into the
Michael> integer-vars. Nothing strange there....

Sure, the RHS is a byte. But devinfo->speed is an int and an lvalue. so
&devinfo->speed is an (int*) (so the cast in this case is a no-op),
and *(int*)&devinfo->speed is the same as *&devinfo->speed which is
the same as devinfo->speed.

But, *(int*)&devinfo->speed is an int no matter what type
devinfo->speed has. So if for some reason, you decide to change the
type of devinfo->speed to a byte, say, the cast will still force
int format data to be stored, overwriting adjacent bits of memory (or
causing an unaligned store trap).

The cast is *wrong*, and potentially dangerous.

I'll submit a patch....

--
Peter C					    peterc@gelato.unsw.edu.au
You are lost in a maze of BitKeeper repositories, all almost the same.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/