Byte Me!

        Submitter: Muli Ben-Yehuda

        Language: C

        Date:        2003/12/20

Recently, I've had the (mis)fortune to work on a driver for an unnamed device. The device in question was little-endian, and as long as the driver was working on Intel platforms (which are little-endian), all was well and good with the world. Except for the usual hardware woes 123, which will be elaborated upon in a separate entry. The day came when we had to port the driver to PowerPC, which is a much nicer architecture in general. Alas, it is also big-endian. This is where the fun starts.

  • The endianness support was added in such a manner that looking at any given value you never knew if it was little-endian or big-endian. No consistency whatsoever. This makes the code, of course, impossible to maintain.
  • In some cases, the conversion depended on the platform the code was running on. In other cases, the conversion needed to be absolute (always convert from little-endian to big-endian, and vice versa). The same macros (hideously named HOST_TO_FOOCARD_XX, FOOCARD_TO_HOST_XX, etc., rather than the sensible Linux cpu_to_le_xx) were used in both cases. This is, of course, bug-prone.
  • This device had registers for programmers to frob. Some of the bits in those registers were reserved (must never be changed by the programmer), and some of them were supposed to be turned on or off occasionally.
An unnamed programmer created masks for those bits:

#define MASK_FOR_SOME_BIT 0x80000000 /* left-most bit */
and then used this mask in her code:
/* read value from register at offset */ 
U32 val = readl(card->someoffset);
val = val | MASK_FOR_SOME_BIT; 
/* write it back */ 
writel(val, card->someoffset);

Can you spot the bug here? Think of the endianness of the value read from the card, and the endianness of the mask, and which bit actually gets changed, and enlightenment shall come.

---------------------------------------------

1 Insufficient documentation.

2 Wrong documentation.

3 No way to figure out why the device doesn't like what we write to it, except for the documentation. See 1 and 2