aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMichal Nazarewicz <mina86@mina86.com>2016-01-15 19:57:58 -0500
committerLinus Torvalds <torvalds@linux-foundation.org>2016-01-16 14:17:22 -0500
commit8f57e4d930d48217268315898212518d4d3e0773 (patch)
tree7a21dc58268fd6dfd9d1399e3765fa0c99aa561e
parentb8a0255db958b8f70c5267dda152e93b6fda1778 (diff)
include/linux/kernel.h: change abs() macro so it uses consistent return type
Rewrite abs() so that its return type does not depend on the architecture and no unexpected type conversion happen inside of it. The only conversion is from unsigned to signed type. char is left as a return type but treated as a signed type regradless of it's actual signedness. With the old version, int arguments were promoted to long and depending on architecture a long argument might result in s64 or long return type (which may or may not be the same). This came after some back and forth with Nicolas. The current macro has different return type (for the same input type) depending on architecture which might be midly iritating. An alternative version would promote to int like so: #define abs(x) __abs_choose_expr(x, long long, \ __abs_choose_expr(x, long, \ __builtin_choose_expr( \ sizeof(x) <= sizeof(int), \ ({ int __x = (x); __x<0?-__x:__x; }), \ ((void)0)))) I have no preference but imagine Linus might. :] Nicolas argument against is that promoting to int causes iconsistent behaviour: int main(void) { unsigned short a = 0, b = 1, c = a - b; unsigned short d = abs(a - b); unsigned short e = abs(c); printf("%u %u\n", d, e); // prints: 1 65535 } Then again, no sane person expects consistent behaviour from C integer arithmetic. ;) Note: __builtin_types_compatible_p(unsigned char, char) is always false, and __builtin_types_compatible_p(signed char, char) is also always false. Signed-off-by: Michal Nazarewicz <mina86@mina86.com> Reviewed-by: Nicolas Pitre <nico@linaro.org> Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> Cc: Wey-Yi Guy <wey-yi.w.guy@intel.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
-rw-r--r--drivers/iio/industrialio-core.c9
-rw-r--r--drivers/net/wireless/intel/iwlwifi/dvm/calib.c2
-rw-r--r--include/linux/kernel.h36
3 files changed, 23 insertions, 24 deletions
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index fd01f3493fc7..af7cc1e65656 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -433,16 +433,15 @@ ssize_t iio_format_value(char *buf, unsigned int type, int size, int *vals)
433 scale_db = true; 433 scale_db = true;
434 case IIO_VAL_INT_PLUS_MICRO: 434 case IIO_VAL_INT_PLUS_MICRO:
435 if (vals[1] < 0) 435 if (vals[1] < 0)
436 return sprintf(buf, "-%ld.%06u%s\n", abs(vals[0]), 436 return sprintf(buf, "-%d.%06u%s\n", abs(vals[0]),
437 -vals[1], 437 -vals[1], scale_db ? " dB" : "");
438 scale_db ? " dB" : "");
439 else 438 else
440 return sprintf(buf, "%d.%06u%s\n", vals[0], vals[1], 439 return sprintf(buf, "%d.%06u%s\n", vals[0], vals[1],
441 scale_db ? " dB" : ""); 440 scale_db ? " dB" : "");
442 case IIO_VAL_INT_PLUS_NANO: 441 case IIO_VAL_INT_PLUS_NANO:
443 if (vals[1] < 0) 442 if (vals[1] < 0)
444 return sprintf(buf, "-%ld.%09u\n", abs(vals[0]), 443 return sprintf(buf, "-%d.%09u\n", abs(vals[0]),
445 -vals[1]); 444 -vals[1]);
446 else 445 else
447 return sprintf(buf, "%d.%09u\n", vals[0], vals[1]); 446 return sprintf(buf, "%d.%09u\n", vals[0], vals[1]);
448 case IIO_VAL_FRACTIONAL: 447 case IIO_VAL_FRACTIONAL:
diff --git a/drivers/net/wireless/intel/iwlwifi/dvm/calib.c b/drivers/net/wireless/intel/iwlwifi/dvm/calib.c
index 07a4c644fb9b..e9cef9de9ed8 100644
--- a/drivers/net/wireless/intel/iwlwifi/dvm/calib.c
+++ b/drivers/net/wireless/intel/iwlwifi/dvm/calib.c
@@ -901,7 +901,7 @@ static void iwlagn_gain_computation(struct iwl_priv *priv,
901 /* bound gain by 2 bits value max, 3rd bit is sign */ 901 /* bound gain by 2 bits value max, 3rd bit is sign */
902 data->delta_gain_code[i] = 902 data->delta_gain_code[i] =
903 min(abs(delta_g), 903 min(abs(delta_g),
904 (long) CHAIN_NOISE_MAX_DELTA_GAIN_CODE); 904 (s32) CHAIN_NOISE_MAX_DELTA_GAIN_CODE);
905 905
906 if (delta_g < 0) 906 if (delta_g < 0)
907 /* 907 /*
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 7311c3294e25..f31638c6e873 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -202,26 +202,26 @@ extern int _cond_resched(void);
202 202
203/** 203/**
204 * abs - return absolute value of an argument 204 * abs - return absolute value of an argument
205 * @x: the value. If it is unsigned type, it is converted to signed type first 205 * @x: the value. If it is unsigned type, it is converted to signed type first.
206 * (s64, long or int depending on its size). 206 * char is treated as if it was signed (regardless of whether it really is)
207 * but the macro's return type is preserved as char.
207 * 208 *
208 * Return: an absolute value of x. If x is 64-bit, macro's return type is s64, 209 * Return: an absolute value of x.
209 * otherwise it is signed long.
210 */ 210 */
211#define abs(x) __builtin_choose_expr(sizeof(x) == sizeof(s64), ({ \ 211#define abs(x) __abs_choose_expr(x, long long, \
212 s64 __x = (x); \ 212 __abs_choose_expr(x, long, \
213 (__x < 0) ? -__x : __x; \ 213 __abs_choose_expr(x, int, \
214 }), ({ \ 214 __abs_choose_expr(x, short, \
215 long ret; \ 215 __abs_choose_expr(x, char, \
216 if (sizeof(x) == sizeof(long)) { \ 216 __builtin_choose_expr( \
217 long __x = (x); \ 217 __builtin_types_compatible_p(typeof(x), char), \
218 ret = (__x < 0) ? -__x : __x; \ 218 (char)({ signed char __x = (x); __x<0?-__x:__x; }), \
219 } else { \ 219 ((void)0)))))))
220 int __x = (x); \ 220
221 ret = (__x < 0) ? -__x : __x; \ 221#define __abs_choose_expr(x, type, other) __builtin_choose_expr( \
222 } \ 222 __builtin_types_compatible_p(typeof(x), signed type) || \
223 ret; \ 223 __builtin_types_compatible_p(typeof(x), unsigned type), \
224 })) 224 ({ signed type __x = (x); __x < 0 ? -__x : __x; }), other)
225 225
226/** 226/**
227 * reciprocal_scale - "scale" a value into range [0, ep_ro) 227 * reciprocal_scale - "scale" a value into range [0, ep_ro)