From: Eric Blake <ebb9@byu.net>
To: Herbert Xu <herbert@gondor.apana.org.au>
Cc: dash@vger.kernel.org
Subject: Re: avoid compiler warning
Date: Tue, 11 Aug 2009 21:30:08 -0600 [thread overview]
Message-ID: <4A823740.1010604@byu.net> (raw)
In-Reply-To: <20090811040330.GE10700@gondor.apana.org.au>
According to Herbert Xu on 8/10/2009 10:03 PM:
> On Thu, Jul 09, 2009 at 12:55:25PM +0000, Eric Blake wrote:
>> ccache gcc -DHAVE_CONFIG_H -I. -I.. -include ../config.h -DBSD=1 -DSHELL
>> -DIFS_BROKEN -Wall -gdwarf-2 -Wall -Werror -MT mystring.o -MD -MP -MF
>> .deps/mystring.Tpo -c -o mystring.o mystring.c
>> miscbltin.c: In function `umaskcmd':
>> miscbltin.c:201: warning: subscript has type `char'
>>
>> isdigit is only defined over EOF and unsigned char values, so without this
>> patch, you can trigger undefined behavior.
>
> What compiler and what libc was this? isdigit is supposed to
> be a function that takes an int argument according to POSIX.
> If libc implements it as a macro then it's up to it to cast
> the parameter to (int).
This is with recent newlib (the warning was intentionally added exactly to
catch the sort of bugs that my patch fixes), coupled with any version of
gcc 3.4 or newer. Additionally, there is a pending bug report against
glibc requesting that glibc add the same QoI warning to flag potentially
buggy code, since it is quite easy to flag the use of raw char as a bug:
http://sources.redhat.com/bugzilla/show_bug.cgi?id=10296
In a particular demonstration of the bug, there are some locales where
isspace('\xff') is false but isspace((unsigned char)'\xff') [aka
isspace(0xff)] is true, when char is a signed type. And although both
glibc and newlib cater to most instances of this bug (as a QoI
enhancement, these libraries guarantee that isspace('\xfe') [aka
isspace(-2)] returns the same result as isspace(0xfe)), not all platforms
have this QoI support, and can actually end up dereferencing outside of
array bounds.
Note that isdigit is a bit unique among the ctype macros: C89 and C99
state that it is locale dependent (with a range still limited to EOF or
[0-UCHAR_MAX]), but POSIX adds the additional restriction that it only
return true for the ten contiguous characters '0' through '9', meaning
that any POSIX-compliant version of isdigit(x) can be as simple as
((unsigned)(x)-'0'<=9) for all x, without regards to locale or
out-of-range arguments. But not all locales comply with POSIX, so it is
not generally portable to rely on isdigit being this simple or fast. On
the other hand, there are a number of packages that #define ISDIGIT,
rather than use ctype's isdigit, exactly to get the speedup guaranteed by
POSIX; this may be something you want to do in dash.
--
Don't work too hard, make some time for fun as well!
Eric Blake ebb9@byu.net
prev parent reply other threads:[~2009-08-12 3:36 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-07-09 12:55 avoid compiler warning Eric Blake
2009-08-11 4:03 ` Herbert Xu
2009-08-11 16:33 ` H. Peter Anvin
2009-08-11 21:56 ` Herbert Xu
2009-08-11 22:06 ` H. Peter Anvin
2009-08-12 3:19 ` Eric Blake
2009-08-31 11:30 ` Eric Blake
2009-08-31 11:31 ` Herbert Xu
2009-08-12 3:32 ` Eric Blake
2009-08-12 3:30 ` Eric Blake [this message]
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=4A823740.1010604@byu.net \
--to=ebb9@byu.net \
--cc=dash@vger.kernel.org \
--cc=herbert@gondor.apana.org.au \
/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