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

Simon Glass sjg at chromium.org
Fri May 11 21:43:50 CEST 2012


Hi,

On Fri, May 11, 2012 at 12:16 PM, Wolfgang Denk <wd at denx.de> wrote:

> 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") ?
>

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 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