>>>>> "Brad" == Brad Chapman <kakadu_croc@yahoo.com> writes:
Brad> OK. The existing API is wrong and the new min()/max() macros
Brad> are the right way to properly compare values. However, we could
Brad> always add a config option to enable a compatibility macro,
Brad> which would use typeof() on one of the two variables and then
Brad> call the real min()/max(). Something like this (just an
Brad> example):
#ifdef CONFIG_ALLOW_COMPAT_MINMAX
#define proper_min(t, a, b) ((t)(a) < (t)(b) ? (a) : (b))
#define proper_max(t, a, b) ((t)(a) > (t)(b) ? (a) : (b))
#define min(a, b) proper_min(typeof(a), a, b)
#define max(a, b) proper_max(typeof(a), a, b)
#else
#define min(t, a, b) ((t)(a) < (t)(b) ? (a) : (b))
#define max(t, a, b) ((t)(a) < (t)(b) ? (a) : (b))
#endif
Conditionals are bad. Casting is bad. In my opinion the only
semantically correct version would be something like this,
#define min(x,y) \
\
({typeof(x) _x = 0; typeof(y) _y = 0; \
if ((_x-1>0 && _y-1<0) || (_x-1<0 && _y-1>0)) { \
if((x) < 0 || (y) < 0) \
exit(SIGNED_UNSIGNED_CONFLICT); \
} \
_x = (x), _y = (y); (_x>_y)?_y:_x; \
})
I think that if you are mixing signed and unsigned you should have had
a check before hand to gaurentee that any casting away of the sign is
ok. It is ok if the variable is in range, it is not if they are out
of range. However, this incurs a runtime check which is not
appropriate for kernel level code.
As several other people have noted, the casting is bad from a
maintainability prospective. This is slapping your best friend in the
face; the compiler. Also, the three arg macro breaks compatibility
with a long standing `C' min/max macro convention.
Probably we should just shutup for now until Linus can clarify what it
is that he wants to achieve and avoid. Since there is probably no way
to make a universally correct min/max macro in `C', you need to have
some sort of goal to achieve.
regards,
Bill Pringlemeir.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Fri Aug 31 2001 - 21:00:20 EST