Just my 2 cents...

On Thu, Nov 12, 2009 at 09:24:00AM +0000, Peter Horton wrote:

> >> +
> >> +#define PEAK_PCI_MAP_SIZE (4 << 10)
> > 
> > I find "0x4000" more readable.
> >
> 
> Maybe, but it'd be wrong, it's 0x1000 :). I write it that way to 
> indicate it's 4K.

I'd say 0x1000 is more common.

> > 
> >> +#define PEAK_PCI_CAN_CLOCK        (8 * 1000 * 1000)
> > 
> > Why not just "8000000"? Or even better (16000000 / 2) to make clear that
> > it's half of the oscillator frequency.
> >
> 
> It's obviously 8 million. 8000000 is more difficult to read, is it 
> 80000000 or 8000000 or 800000?

I agree with Peter here and I have seen such constructs throughout the kernel.
As the compiler will make the same out of it anyway, I'd vote for the
unambiguous way.

> >> +
> >> +static u8 peak_pci_can_read(struct sja1000_priv const *ctlr, int reg)
> > 
> > "const struct sja1000_priv" please.
> >
> 
> It's easier to see where the "const" applies this way, you just read 
> from right to left. It makes the difference between "struct x * const" 
> and "struct x const *" more obvious.

I think Wolfgang's proposal is pretty much standard in the kernel.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Attachment: signature.asc
Description: Digital signature

_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core

Reply via email to