[PATCH] thermal: Fix KELVIN_TO_CELSIUS macro

[PATCH] thermal: Fix KELVIN_TO_CELSIUS macro

[Guenter Roeck]

It is always a good idea to use paranthesis around macro parameters
to avoid undesired side effects.

In this specific case, KELVIN_TO_CELSIUS() is used in
drivers/platform/x86/asus-wmi.c with parameter "value & 0xFFFF",
which due to operator evaluation order causes more or less random
results.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 include/linux/thermal.h |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index f7e11c7..02488c9 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -41,8 +41,8 @@
 #define THERMAL_NO_LIMIT	THERMAL_CSTATE_INVALID
 
 /* Unit conversion macros */
-#define KELVIN_TO_CELSIUS(t)	(long)(((long)t-2732 >= 0) ?	\
-				((long)t-2732+5)/10 : ((long)t-2732-5)/10)
+#define KELVIN_TO_CELSIUS(t)	(long)(((long)(t)-2732 >= 0) ?	\
+				((long)(t)-2732+5)/10 : ((long)(t)-2732-5)/10)
 #define CELSIUS_TO_KELVIN(t)	((t)*10+2732)
 
 /* Adding event notification support elements */
-- 
1.7.9.7

Re: [PATCH] thermal: Fix KELVIN_TO_CELSIUS macro

[Joe Perches]

On Sun, 2014-03-23 at 21:14 -0700, Guenter Roeck wrote:
> It is always a good idea to use paranthesis around macro parameters
> to avoid undesired side effects.
> 
> In this specific case, KELVIN_TO_CELSIUS() is used in
> drivers/platform/x86/asus-wmi.c with parameter "value & 0xFFFF",
> which due to operator evaluation order causes more or less random
> results.

Maybe it's better to use a statement expression to avoid
multiple calculations of t

Maybe;

#define KELVIN_TO_CELSIUS(t)				\
({							\
	long _t = (long)(t) - 2732;			\
	_t >= 0 ? (_t + 5) / 10 : (_t - 5) / 10;	\
})

Re: [PATCH] thermal: Fix KELVIN_TO_CELSIUS macro

[Guenter Roeck]

On 03/23/2014 09:25 PM, Joe Perches wrote:
> On Sun, 2014-03-23 at 21:14 -0700, Guenter Roeck wrote:
>> It is always a good idea to use paranthesis around macro parameters
>> to avoid undesired side effects.
>>
>> In this specific case, KELVIN_TO_CELSIUS() is used in
>> drivers/platform/x86/asus-wmi.c with parameter "value & 0xFFFF",
>> which due to operator evaluation order causes more or less random
>> results.
>
> Maybe it's better to use a statement expression to avoid
> multiple calculations of t
>
> Maybe;
>
> #define KELVIN_TO_CELSIUS(t)				\
> ({							\
> 	long _t = (long)(t) - 2732;			\
> 	_t >= 0 ? (_t + 5) / 10 : (_t - 5) / 10;	\
> })
>
>

Possibly, but I only want to have a bug fixed which should possibly be applied
to -stable at some point. As such, I prefer the minimalistic approach.
Anything more fancy on top of that can be submitted as separate patch.

Guenter

Re: [PATCH] thermal: Fix KELVIN_TO_CELSIUS macro

[Zhang Rui]

On Sun, 2014-03-23 at 22:30 -0700, Guenter Roeck wrote:
> On 03/23/2014 09:25 PM, Joe Perches wrote:
> > On Sun, 2014-03-23 at 21:14 -0700, Guenter Roeck wrote:
> >> It is always a good idea to use paranthesis around macro parameters
> >> to avoid undesired side effects.
> >>
> >> In this specific case, KELVIN_TO_CELSIUS() is used in
> >> drivers/platform/x86/asus-wmi.c with parameter "value & 0xFFFF",
> >> which due to operator evaluation order causes more or less random
> >> results.
> >
> > Maybe it's better to use a statement expression to avoid
> > multiple calculations of t
> >
> > Maybe;
> >
> > #define KELVIN_TO_CELSIUS(t)				\
> > ({							\
> > 	long _t = (long)(t) - 2732;			\
> > 	_t >= 0 ? (_t + 5) / 10 : (_t - 5) / 10;	\
> > })
> >
> >
> 
> Possibly, but I only want to have a bug fixed which should possibly be applied
> to -stable at some point. As such, I prefer the minimalistic approach.
> Anything more fancy on top of that can be submitted as separate patch.
> 
I agreed. I will take this patch and push it for stable kernel.

thanks,
rui