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

[Willy Tarreau <willy@w.ods.org>]

On Wed, Jun 22, 2005 at 03:59:12PM -0500, Bill Gatliff wrote:
 
> 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.

Bill, this is a good reason, I agree with you. What I did not want to see
was something like :

  enum { TCLK_BASE=0xa80, TCLK_REG0, TCLK_REG1, ... }

> 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...

indeed, nearly all defines need to get lots of parenthesis for the exact
same reason.

Thanks for pointing out the gdb trick.
Willy