Subject: Re: FROMBCD coding style flaw
To: Anders Gavare <anders@gavare.se>
From: Martin Husemann <martin@duskware.de>
List: port-prep
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