Re: Patch of a new driver for kernel 2.4.x that need review

From: Bill Gatliff
Date: Wed Jun 22 2005 - 16:08:17 EST


Willy Tarreau wrote:


On Wed, Jun 22, 2005 at 10:43:40PM +0300, Pekka Enberg wrote:

On 6/22/05, Bouchard, Sebastien <Sebastien.Bouchard@xxxxxxxxxxxxxx> wrote:


Please use enums instead.

I dont agree with you here : enums are good to simply specify an ordering.
But they must not be used to specify static mapping. Eg: if REG4 *must* be
equal to BASE+4, you should not use enums, otherwise it will render the
code unreadable. I personnaly don't want to count the position of REG7 in
the enum to discover that it's at BASE+7.

What Sebastien is after is something like this:

enum tclk_regid {TCLK_BASE=0xa80, TCLK_REG0=TCLK_BASE, TCLK_REG1=TCLK_BASE+1...};
enum tclk_regid tclk;

And then later on, if you ask gdb with the value of tclk is, it can tell you "TCLK_REG1", instead of just 0xa801. You can also assign values to tclk from within gdb using the enumerations, rather than magic numbers.

If you insist on using #defines, then you need to do them like this at the very least:

#define TCLK_REG7 (TCLK_BASE+7)

... so as to prevent operator precedence problems later on. I.e. what happens here:

tclk = TCLK_REG7 / 2;

Not implying that the above is a realistic example, I'm just pointing out a potential gotcha that is easily avoided...


Bill Gatliff
"A DTI spokesman said Wicks would use his debut announcement to
'reaffirm the government's commitment to developing wind' to tackle
the threat of climate change." -- The Observer, May 22, 2005

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at