From: Alan Stern <[EMAIL PROTECTED]>

    > > -               ptr[4] = MSB_of(info->pagesize>>16);
    > > -               ptr[5] = LSB_of(info->pagesize>>16);
    > > -               ptr[6] = MSB_of(info->pagesize&0xFFFF);
    > > -               ptr[7] = LSB_of(info->pagesize&0xFFFF);
    > > +               ((u32 *) ptr)[1] = cpu_to_be32(info->pagesize);
    > > 
    > > A good compiler generates the same code, and for a human
    > > like me the above four statements show much more clearly
    > > that values are assigned to ptr[4..7] than the ugly cast below.

    My opinion is slightly mixed.  I definitely think the old way is too
    repetitive, unclear, and error-prone.  On the other hand, I also dislike
    the ugly cast.  A possible alternative, also ugly ...

Let me tell you - I have programmed for twenty years without
knowing the meaning of little-endian and big-endian. Yes,
they had something to do with the order of bytes in an integer,
but there are many strange architectures and mixed versions,
and there is no need at all to know such things.
Yes, knowing such things is directly harmful.

Seeing such a cast must raise an alarm. Caution! Bug ahead!
And indeed, I glanced over these patches and saw two casts,
one of them buggy. 50%.

If one has a char * then only use it to read or write chars.
If one has an int * then only use it to read or write ints.
Never use a cast.

There are even architectures where the size of a char * differs
from that of an int *. One never knows whether the compiler is going
to change the bit pattern upon such a cast or not.

And then there are alignment restrictions.
If you really want to write one line of obscure code that
replaces 4 byte assignments, then the reader gets an additional
obligation, namely to check that this char * is suitably aligned
to be abused as int *.

Altogether ugliness. Forgivable in case efficiency matters,
and it really does make a difference. Not elsewhere.

Andries



-------------------------------------------------------
This SF.net email is sponsored by: Perforce Software.
Perforce is the Fast Software Configuration Management System offering
advanced branching capabilities and atomic changes on 50+ platforms.
Free Eval! http://www.perforce.com/perforce/loadprog.html
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to