[RFC][PATCH] fix abs() macro to work with types wider than int

[RFC][PATCH] fix abs() macro to work with types wider than int

[Jiri Bohac]

Hi,

is there any reason to use an explicit int instead of a typeof in
the abs() macro? The current implementation will return bogus
results when used with longs.

How about changing the int to a typeof like this?:





Fix the abs() macro to work with wider types than int.

Signed-off-by: Jiri Bohac <jbohac@suse.cz>

--- linux-2.6.21-rc5.orig/include/linux/kernel.h
+++ linux-2.6.21-rc5/include/linux/kernel.h
@@ -89,7 +89,7 @@ extern int cond_resched(void);
 #define might_sleep_if(cond) do { if (cond) might_sleep(); } while (0)
 
 #define abs(x) ({				\
-		int __x = (x);			\
+		typeof(x) __x = (x);			\
 		(__x < 0) ? -__x : __x;		\
 	})
 



-- 
Jiri Bohac <jbohac@suse.cz>
SUSE Labs, SUSE CZ

Re: [RFC][PATCH] fix abs() macro to work with types wider than int

[Randy Dunlap]

On Thu, 19 Apr 2007 11:23:39 +0200 Jiri Bohac wrote:

> Hi,
> 
> is there any reason to use an explicit int instead of a typeof in
> the abs() macro? The current implementation will return bogus
> results when used with longs.

I think it's like it is just to be consistent with abs() in C,
which also contains labs() and llabs().

> How about changing the int to a typeof like this?:
> 
> 
> 
> Fix the abs() macro to work with wider types than int.
> 
> Signed-off-by: Jiri Bohac <jbohac@suse.cz>
> 
> --- linux-2.6.21-rc5.orig/include/linux/kernel.h
> +++ linux-2.6.21-rc5/include/linux/kernel.h
> @@ -89,7 +89,7 @@ extern int cond_resched(void);
>  #define might_sleep_if(cond) do { if (cond) might_sleep(); } while (0)
>  
>  #define abs(x) ({				\
> -		int __x = (x);			\
> +		typeof(x) __x = (x);			\
>  		(__x < 0) ? -__x : __x;		\
>  	})
>  
> 
> 
> 
> -- 
> Jiri Bohac <jbohac@suse.cz>
> SUSE Labs, SUSE CZ

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [RFC][PATCH] fix abs() macro to work with types wider than int

[Richard Knutsson]

Randy Dunlap wrote:
> On Thu, 19 Apr 2007 11:23:39 +0200 Jiri Bohac wrote:
>
>   
>> Hi,
>>
>> is there any reason to use an explicit int instead of a typeof in
>> the abs() macro? The current implementation will return bogus
>> results when used with longs.
>>     
>
> I think it's like it is just to be consistent with abs() in C,
> which also contains labs() and llabs().
>   
We actually had labs() before (few months ago), but since it was not 
used, and if it would it seemed better to just fix abs(), it was 
removed. So I think this is the appropriate way to go.
>   
>> How about changing the int to a typeof like this?:
>>
>>
>>
>> Fix the abs() macro to work with wider types than int.
>>
>> Signed-off-by: Jiri Bohac <jbohac@suse.cz>
>>
>> --- linux-2.6.21-rc5.orig/include/linux/kernel.h
>> +++ linux-2.6.21-rc5/include/linux/kernel.h
>> @@ -89,7 +89,7 @@ extern int cond_resched(void);
>>  #define might_sleep_if(cond) do { if (cond) might_sleep(); } while (0)
>>  
>>  #define abs(x) ({				\
>> -		int __x = (x);			\
>> +		typeof(x) __x = (x);			\
>>  		(__x < 0) ? -__x : __x;		\
>>  	})
>>  
>>
>>
>>     

Re: [RFC][PATCH] fix abs() macro to work with types wider than int

[Randy Dunlap]

On Fri, 20 Apr 2007 14:44:38 +0200 Richard Knutsson wrote:

> Randy Dunlap wrote:
> > On Thu, 19 Apr 2007 11:23:39 +0200 Jiri Bohac wrote:
> >
> >   
> >> Hi,
> >>
> >> is there any reason to use an explicit int instead of a typeof in
> >> the abs() macro? The current implementation will return bogus
> >> results when used with longs.
> >>     
> >
> > I think it's like it is just to be consistent with abs() in C,
> > which also contains labs() and llabs().
> >   
> We actually had labs() before (few months ago), but since it was not 
> used, and if it would it seemed better to just fix abs(), it was 
> removed. So I think this is the appropriate way to go.

Sounds like when someone actually needs labs() or llabs()
they can submit a patch for however they would like to use it.

> >> How about changing the int to a typeof like this?:
> >>
> >>
> >>
> >> Fix the abs() macro to work with wider types than int.
> >>
> >> Signed-off-by: Jiri Bohac <jbohac@suse.cz>
> >>
> >> --- linux-2.6.21-rc5.orig/include/linux/kernel.h
> >> +++ linux-2.6.21-rc5/include/linux/kernel.h
> >> @@ -89,7 +89,7 @@ extern int cond_resched(void);
> >>  #define might_sleep_if(cond) do { if (cond) might_sleep(); } while (0)
> >>  
> >>  #define abs(x) ({				\
> >> -		int __x = (x);			\
> >> +		typeof(x) __x = (x);			\
> >>  		(__x < 0) ? -__x : __x;		\
> >>  	})


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [RFC][PATCH] fix abs() macro to work with types wider than int

[John Anthony Kazos Jr.]

> > > I think it's like it is just to be consistent with abs() in C,
> > > which also contains labs() and llabs().
> > >   
> > We actually had labs() before (few months ago), but since it was not 
> > used, and if it would it seemed better to just fix abs(), it was 
> > removed. So I think this is the appropriate way to go.
> 
> Sounds like when someone actually needs labs() or llabs()
> they can submit a patch for however they would like to use it.

However they would like to use *abs()? What different ways are possible to 
take the arithmetic absolute value? I see record of many cases where 
dozens of authors have macros that then get collapsed to include files. So 
why not avoid that annoyance this time and -start- with it in the include 
files?

Can there even be any reason beyond unnecessary pedantics to have 
[l[l]]abs?

Re: [RFC][PATCH] fix abs() macro to work with types wider than int

[Randy Dunlap]

John Anthony Kazos Jr. wrote:
>>>> I think it's like it is just to be consistent with abs() in C,
>>>> which also contains labs() and llabs().
>>>>   
>>> We actually had labs() before (few months ago), but since it was not 
>>> used, and if it would it seemed better to just fix abs(), it was 
>>> removed. So I think this is the appropriate way to go.
>> Sounds like when someone actually needs labs() or llabs()
>> they can submit a patch for however they would like to use it.
> 
> However they would like to use *abs()? What different ways are possible to 
> take the arithmetic absolute value? I see record of many cases where 
> dozens of authors have macros that then get collapsed to include files. So 
> why not avoid that annoyance this time and -start- with it in the include 
> files?
> 
> Can there even be any reason beyond unnecessary pedantics to have 
> [l[l]]abs?

See Paragraph 1 above.  We do lots of functions in a manner that is
like C (or libc) so that we don't confuse developers.

-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [RFC][PATCH] fix abs() macro to work with types wider than int

[linux-os (Dick Johnson)]

On Wed, 25 Apr 2007, John Anthony Kazos Jr. wrote:

>>>> I think it's like it is just to be consistent with abs() in C,
>>>> which also contains labs() and llabs().
>>>>
>>> We actually had labs() before (few months ago), but since it was not
>>> used, and if it would it seemed better to just fix abs(), it was
>>> removed. So I think this is the appropriate way to go.
>>
>> Sounds like when someone actually needs labs() or llabs()
>> they can submit a patch for however they would like to use it.
>
> However they would like to use *abs()? What different ways are possible to
> take the arithmetic absolute value? I see record of many cases where
> dozens of authors have macros that then get collapsed to include files. So
> why not avoid that annoyance this time and -start- with it in the include
> files?
>
> Can there even be any reason beyond unnecessary pedantics to have
> [l[l]]abs?

Isn't any macro that looks for "< 0" insensitive to the word length? I think 
it's only when people try tricks with "sign-bits," that they get into trouble.
I think this works, regardless of the length of the integers:

#define abs(x) (((x)<0)?-(x):(x))



Cheers,
Dick Johnson
Penguin : Linux version 2.6.16.24 on an i686 machine (5592.62 BogoMips).
New book: http://www.AbominableFirebug.com/
_
\x1a\x04

****************************************************************
The information transmitted in this message is confidential and may be privileged.  Any review, retransmission, dissemination, or other use of this information by persons or entities other than the intended recipient is prohibited.  If you are not the intended recipient, please notify Analogic Corporation immediately - by replying to this message or by sending an email to DeliveryErrors@analogic.com - and destroy all copies of this information, including any attachments, without reading or disclosing them.

Thank you.

Re: [RFC][PATCH] fix abs() macro to work with types wider than int

[Andreas Schwab]

"linux-os (Dick Johnson)" <linux-os@analogic.com> writes:

> I think this works, regardless of the length of the integers:
>
> #define abs(x) (((x)<0)?-(x):(x))

But it evaluates its argument more than once.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [RFC][PATCH] fix abs() macro to work with types wider than int

[linux-os (Dick Johnson)]

On Wed, 25 Apr 2007, Andreas Schwab wrote:

> "linux-os (Dick Johnson)" <linux-os@analogic.com> writes:
>
>> I think this works, regardless of the length of the integers:
>>
>> #define abs(x) (((x)<0)?-(x):(x))
>
> But it evaluates its argument more than once.
>
> Andreas.
>

Sure. Macros do that. If that's not okay, then you need either
a temporary variable or a function. The original query was for
a macro. Any macro that tests something then returns a value
based upon the test is going to have that problem. Any attempt
to just AND off a sign bit is going to have width problems. You
can't have your cake and eat it too. Even whacking off the sign-
bit may fail because there may be two variable accesses which could
mean two function calls, destroying the logic.


Cheers,
Dick Johnson
Penguin : Linux version 2.6.16.24 on an i686 machine (5592.62 BogoMips).
New book: http://www.AbominableFirebug.com/
_
\x1a\x04

****************************************************************
The information transmitted in this message is confidential and may be privileged.  Any review, retransmission, dissemination, or other use of this information by persons or entities other than the intended recipient is prohibited.  If you are not the intended recipient, please notify Analogic Corporation immediately - by replying to this message or by sending an email to DeliveryErrors@analogic.com - and destroy all copies of this information, including any attachments, without reading or disclosing them.

Thank you.

Re: [RFC][PATCH] fix abs() macro to work with types wider than int

[Andreas Schwab]

"linux-os (Dick Johnson)" <linux-os@analogic.com> writes:

> The original query was for a macro.

A macro that does not evaluate its argument more than once.  If you want
it to match the behaviour as defined by the C standard then it is not
allowed to do so.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [RFC][PATCH] fix abs() macro to work with types wider than int

[John Anthony Kazos Jr.]

> > Can there even be any reason beyond unnecessary pedantics to have [l[l]]abs?
> 
> See Paragraph 1 above.  We do lots of functions in a manner that is
> like C (or libc) so that we don't confuse developers.

I have the perfect solution that includes compatibility while avoiding 
stupidity.

Macro "abs" which evaluates its argument once and uses typeof().

#define labs(n) abs(n)
#define llabs(n) labs(n)

Leave that in the kernel for a year or two. At the end of that time, grep 
the kernel to show that only two silly people actually use them. Change 
those, and remove the aliases. Sense restored.

As far as I'm concerned, the only reason those functions exist like that 
is because standard C doesn't support function overloading and doesn't 
want to promote every argument to the biggest width and -also- can't 
define it as a macro because there's no typeof() in standard C. So since 
we do have it...

...and why do we want people hacking the kernel who get confused by a 
single "abs" function? Isn't that quite a lot like saying "Do not use 
hairdryer while sleeping."?