[U-Boot] [PATCH v7 03/23] Add abs() macro to return absolute value

Wolfgang Denk wd at denx.de
Fri May 11 21:16:26 CEST 2012


Dear Simon Glass,

In message <1336685875-13177-4-git-send-email-sjg at 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") ?

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 at denx.de
It seems intuitively obvious to me, which  means  that  it  might  be
wrong.                                                 -- Chris Torek


More information about the U-Boot mailing list