I was just digging a bit around in linux/kernel.h and stumbled over the abs()
makro. For me it looks as it would return wrong results on 64 bit platforms
if the input value is greater than 2^32.
Signed-off-by: Rolf Eike Beer <[email protected]>
---
include/linux/kernel.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index d6320a3..1e6eb66 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -145,7 +145,7 @@ extern int _cond_resched(void);
#define might_sleep_if(cond) do { if (cond) might_sleep(); } while (0)
#define abs(x) ({ \
- int __x = (x); \
+ long __x = (x); \
(__x < 0) ? -__x : __x; \
})
--
1.6.0.2
On Thu, 3 Sep 2009 23:12:01 +0200
Rolf Eike Beer <[email protected]> wrote:
> I was just digging a bit around in linux/kernel.h and stumbled over the abs()
> makro. For me it looks as it would return wrong results on 64 bit platforms
> if the input value is greater than 2^32.
>
Yes, it will truncate.
>
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index d6320a3..1e6eb66 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -145,7 +145,7 @@ extern int _cond_resched(void);
> #define might_sleep_if(cond) do { if (cond) might_sleep(); } while (0)
>
> #define abs(x) ({ \
> - int __x = (x); \
> + long __x = (x); \
> (__x < 0) ? -__x : __x; \
> })
I wonder if that ends up producing worse code in the normal case.
We could use typeof to address that but what about
unsigned foo = -1;
signed bar = abs(foo);
?
That'll currently return 1 but if we use typeof it'll return
0xffffffffU which gets turned into -1.
hrm.
Andrew Morton wrote:
> On Thu, 3 Sep 2009 23:12:01 +0200
>
> Rolf Eike Beer <[email protected]> wrote:
> > I was just digging a bit around in linux/kernel.h and stumbled over the
> > abs() makro. For me it looks as it would return wrong results on 64 bit
> > platforms if the input value is greater than 2^32.
>
> Yes, it will truncate.
>
> > diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> > index d6320a3..1e6eb66 100644
> > --- a/include/linux/kernel.h
> > +++ b/include/linux/kernel.h
> > @@ -145,7 +145,7 @@ extern int _cond_resched(void);
> > #define might_sleep_if(cond) do { if (cond) might_sleep(); } while (0)
> >
> > #define abs(x) ({ \
> > - int __x = (x); \
> > + long __x = (x); \
> > (__x < 0) ? -__x : __x; \
> > })
>
> I wonder if that ends up producing worse code in the normal case.
>
> We could use typeof to address that but what about
>
> unsigned foo = -1;
> signed bar = abs(foo);
>
> ?
>
> That'll currently return 1 but if we use typeof it'll return
> 0xffffffffU which gets turned into -1.
I have not tested this, but what might happen with:
unsigned long a = 7;
long b = 9;
return abs(a - b);
I thought about using typeof but that's too much compiler magic for me to get
right. If you can, go for it.
We could also try to check sizeof(__x) and then use either int or long to get
the same code for the common code but behave right for big values.
Eike
On Thu, 3 Sep 2009, Rolf Eike Beer wrote:
> I was just digging a bit around in linux/kernel.h and stumbled over the abs()
> makro. For me it looks as it would return wrong results on 64 bit platforms
> if the input value is greater than 2^32.
>
I think you are right.
> Signed-off-by: Rolf Eike Beer <[email protected]>
> ---
> include/linux/kernel.h | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index d6320a3..1e6eb66 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -145,7 +145,7 @@ extern int _cond_resched(void);
> #define might_sleep_if(cond) do { if (cond) might_sleep(); } while (0)
>
> #define abs(x) ({ \
> - int __x = (x); \
> + long __x = (x); \
> (__x < 0) ? -__x : __x; \
> })
>
>
--
Jesper Juhl <[email protected]> http://www.chaosbits.net/
Plain text mails only, please http://www.expita.com/nomime.html
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html