Subject: Re: FROMBCD coding style flaw
To: Anders Gavare <anders@gavare.se>
From: Martin Husemann <martin@duskware.de>
List: tech-kern
Date: 11/20/2005 17:44:31
On Sun, Nov 20, 2005 at 04:16:41PM +0100, Anders Gavare wrote:
> Basically, the FROMBCD macro is misused if it is given a function call
> as an argument, instead of a plain variable.

A quick idutils run showed at least the following broken uses:

arch/evbarm/tsarm/tsrtc.c
arch/next68k/next68k/rtc.c
arch/prep/isa/mcclock_isa.c
arch/sh3/sh3/clock.c
dev/ic/mm58167.c

IMHO the callers should be fixed, but the question is how to make sure
we catch them.

Maybe we could define a different version for DIAGNOSTIC or DEBUG kernels
that takes the address of the arg, so it fails compilation for the broken
usage - something like this (untested):

#if defined(__GNUC__) && defined(DIAGNOSTIC)
#define     FROMBCD(x)      ({				\
		__typeof(x) *v = &x;			\
		(((*v) >> 4) * 10 + ((*v) & 0xf));	\
	}
#else
#define     FROMBCD(x)      (((x) >> 4) * 10 + ((x) & 0xf))
#endif


And I think we should eliminate all duplicate definitions of this macro,
no matter what solution we go for, which seem to happen at least in:

arch/mvme68k/stand/libsa/chiptotime.c
arch/mvme68k/stand/libsa/clock.c
arch/mvmeppc/stand/libsa/clock.c

Martin