Re: FROMBCD coding style flaw

Martin Husemann <[email protected]>
Newsgroups gmane.os.netbsd.devel.kernel,gmane.os.netbsd.ports.prep
Message-ID <[email protected]>
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.