[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sun, 26 Jul 2015 12:53:25 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: Left-shift of negative number
On Sat, Jul 25, 2015 at 09:26:34PM +0300, Alexander Cherepanov wrote:
> >I've gone ahead and made the change as commit
> >fe7582f4f92152ab60e9523bf146fe28ceae51f6. If anything looks wrong,
> >please let me know. Thanks again for the bug report.
>
> The new definition of R:
>
> #define R(a,b) ((uint32_t)((a==0x80 ? 0x40u-b : 0u-a) << 23))
>
> It implicitly casts a and b to unsigned (and triggers
> -Wsign-conversion). Isn't it better to express it explicitly, e.g.
Using implicit conversion to unsigned types is idiomatic in musl; we
don't use -Wsign-comparison or similar in the default CFLAGS. A
discussion about changing that should be library-global, not specific
to this one piece of code, but I'd rather not anyway. My view is that
there's a tradeoff between using implicit conversions and casts: using
the implicit conversions (and leaving off warnings for them) could
hide bugs from wrong expectations about types, but using casts can
hide much more dangerous conversions for which an implicit conversion
would not even be possible. I generally like to keep the number of
casts to a minimum.
> by moving the cast to uint32_t inside the conditional operator? Or
> maybe more intuitive to move the work with negative numbers outside
> the conditional operator:
>
> #define R(a,b) (-(uint32_t)(a==0x80 ? b-0x40 : a) << 23)
This actually would give the wrong result -- and strictly speaking,
even worse, undefined behavior -- if uint32_t had lower rank than int.
While other parts of musl assume 32-bit int for Linux-specific reasons
(advanced parts of the futex API, etc.) I'd rather leave this code
only assuming that int is >=32bit rather than exactly 32-bit.
Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
