Re: [PATCH v2] Add kabs()

[Nick Bowler]

On 2012-04-23 16:22 +0300, Alexey Dobriyan wrote:
> There is abs() and abs64().
> They have following unwanted or suprising properties:
[...]
> 2) In finest Unix tradition they do not work reliably.
>    Quoting abs(3).
> 	"Trying to take the absolute value of the most negative integer is not defined."

Unforunately, your version does not actually fix this problem,
because...

> +#define kabs(x)								\
> +({									\
> +	typeof(x) _x = (x);						\
> +									\
[...]
> +		__builtin_types_compatible_p(typeof(_x), int),		\
> +		(unsigned int)({ _x < 0 ? -_x : _x; }),			\

... here, if x == INT_MIN, then evaluating -x results in signed overflow
and thus undefined behaviour.  The problem is that the conversion to
unsigned int happens too late.

It needs to be (untested):

  _x == INT_MIN ? (unsigned)_x : (unsigned)(_x < 0 ? -_x : _x)

This version assumes a couple things about the representation of
signed/unsigned types, in particular that INT_MIN is exactly one less
than -INT_MAX, and that UINT_MAX is exactly one more than 2u*INT_MAX.
I presume this is true for everything that Linux targets.

Ditto for the other types, although types "smaller" than int will be
promoted to int before being negated, so the overflow probably won't
happen for them.

[...]
> +	_x))))));							\
> +})

Cheers,
-- 
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/