Commit 8f57e4d9 authored by Michal Nazarewicz's avatar Michal Nazarewicz Committed by Linus Torvalds

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: default avatarMichal Nazarewicz <mina86@mina86.com>
Reviewed-by: default avatarNicolas 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: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent b8a0255d
...@@ -433,15 +433,14 @@ ssize_t iio_format_value(char *buf, unsigned int type, int size, int *vals) ...@@ -433,15 +433,14 @@ ssize_t iio_format_value(char *buf, unsigned int type, int size, int *vals)
scale_db = true; scale_db = true;
case IIO_VAL_INT_PLUS_MICRO: case IIO_VAL_INT_PLUS_MICRO:
if (vals[1] < 0) if (vals[1] < 0)
return sprintf(buf, "-%ld.%06u%s\n", abs(vals[0]), return sprintf(buf, "-%d.%06u%s\n", abs(vals[0]),
-vals[1], -vals[1], scale_db ? " dB" : "");
scale_db ? " dB" : "");
else else
return sprintf(buf, "%d.%06u%s\n", vals[0], vals[1], return sprintf(buf, "%d.%06u%s\n", vals[0], vals[1],
scale_db ? " dB" : ""); scale_db ? " dB" : "");
case IIO_VAL_INT_PLUS_NANO: case IIO_VAL_INT_PLUS_NANO:
if (vals[1] < 0) if (vals[1] < 0)
return sprintf(buf, "-%ld.%09u\n", abs(vals[0]), return sprintf(buf, "-%d.%09u\n", abs(vals[0]),
-vals[1]); -vals[1]);
else else
return sprintf(buf, "%d.%09u\n", vals[0], vals[1]); return sprintf(buf, "%d.%09u\n", vals[0], vals[1]);
......
...@@ -901,7 +901,7 @@ static void iwlagn_gain_computation(struct iwl_priv *priv, ...@@ -901,7 +901,7 @@ static void iwlagn_gain_computation(struct iwl_priv *priv,
/* bound gain by 2 bits value max, 3rd bit is sign */ /* bound gain by 2 bits value max, 3rd bit is sign */
data->delta_gain_code[i] = data->delta_gain_code[i] =
min(abs(delta_g), min(abs(delta_g),
(long) CHAIN_NOISE_MAX_DELTA_GAIN_CODE); (s32) CHAIN_NOISE_MAX_DELTA_GAIN_CODE);
if (delta_g < 0) if (delta_g < 0)
/* /*
......
...@@ -202,26 +202,26 @@ extern int _cond_resched(void); ...@@ -202,26 +202,26 @@ extern int _cond_resched(void);
/** /**
* abs - return absolute value of an argument * abs - return absolute value of an argument
* @x: the value. If it is unsigned type, it is converted to signed type first * @x: the value. If it is unsigned type, it is converted to signed type first.
* (s64, long or int depending on its size). * char is treated as if it was signed (regardless of whether it really is)
* but the macro's return type is preserved as char.
* *
* Return: an absolute value of x. If x is 64-bit, macro's return type is s64, * Return: an absolute value of x.
* otherwise it is signed long.
*/ */
#define abs(x) __builtin_choose_expr(sizeof(x) == sizeof(s64), ({ \ #define abs(x) __abs_choose_expr(x, long long, \
s64 __x = (x); \ __abs_choose_expr(x, long, \
(__x < 0) ? -__x : __x; \ __abs_choose_expr(x, int, \
}), ({ \ __abs_choose_expr(x, short, \
long ret; \ __abs_choose_expr(x, char, \
if (sizeof(x) == sizeof(long)) { \ __builtin_choose_expr( \
long __x = (x); \ __builtin_types_compatible_p(typeof(x), char), \
ret = (__x < 0) ? -__x : __x; \ (char)({ signed char __x = (x); __x<0?-__x:__x; }), \
} else { \ ((void)0)))))))
int __x = (x); \
ret = (__x < 0) ? -__x : __x; \ #define __abs_choose_expr(x, type, other) __builtin_choose_expr( \
} \ __builtin_types_compatible_p(typeof(x), signed type) || \
ret; \ __builtin_types_compatible_p(typeof(x), unsigned type), \
})) ({ signed type __x = (x); __x < 0 ? -__x : __x; }), other)
/** /**
* reciprocal_scale - "scale" a value into range [0, ep_ro) * reciprocal_scale - "scale" a value into range [0, ep_ro)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment