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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Willy:

Willy Tarreau wrote:

Hi,

On Wed, Jun 22, 2005 at 10:43:40PM +0300, Pekka Enberg wrote:
On 6/22/05, Bouchard, Sebastien <[email protected]> wrote:
+#define TLCLK_REG7 TLCLK_BASE+7
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...



b.g.

--
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
[email protected]

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux