From: Nick Bowler <nbowler@elliptictech.com>
To: Alexey Dobriyan <adobriyan@gmail.com>
Cc: akpm@linux-foundation.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] Add kabs()
Date: Mon, 23 Apr 2012 14:18:53 -0400 [thread overview]
Message-ID: <20120423181853.GA13863@elliptictech.com> (raw)
In-Reply-To: <20120423132206.GA2516@p183.telecom.by>
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/)
next prev parent reply other threads:[~2012-04-23 18:18 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-04-23 13:11 [PATCH] Add kabs() Alexey Dobriyan
2012-04-23 13:22 ` [PATCH v2] " Alexey Dobriyan
2012-04-23 18:18 ` Nick Bowler [this message]
2012-04-25 17:20 ` [PATCH] " Ben Pfaff
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120423181853.GA13863@elliptictech.com \
--to=nbowler@elliptictech.com \
--cc=adobriyan@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox