Re: Strange code in ide_cdrom_register

[Peter Chubb]

>>>>> "Michael" == Michael Dunsky <[e-mail]> 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					    [e-mail]
You are lost in a maze of BitKeeper repositories, all almost the same.