
Hi,
On Fri, May 11, 2012 at 12:16 PM, Wolfgang Denk wd@denx.de wrote:
Dear Simon Glass,
In message 1336685875-13177-4-git-send-email-sjg@chromium.org you wrote:
This macro is generally useful to make it available in common.
I agree with the idea of the patch, but I object against the current implementation.
+/* Return the absolute value of a number */ +#define abs(x) ((x) < 0 ? -(x) : (x))
NAK. This macro can have nasty side effects. Consider something like
foo = abs(bar++);
or
foo = abs(in_be32(reg));
etc.
Why do you re-invent the wheel (poorly) instead of - for example - copying the definition from Linux ("include/linux/kernel.h") ?
That was the similar to the first implementation, which always returned long, but Albert didn't like it do I did something simple.
So I will go back to the more complicated way, and this time just copy what the kernel does. The difference is that it doesn't use typeof().
Regards, Simon
Best regards,
Wolfgang Denk
-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de It seems intuitively obvious to me, which means that it might be wrong. -- Chris Torek