[Buildroot] [PATCH 05/11] dialog: Patch incorrect use of toupper()

Thomas De Schampheleire patrickdepinguin at gmail.com
Sat Jul 19 14:06:48 UTC 2014


Hi,

On Thu, Apr 10, 2014 at 11:44 AM, Arnout Vandecappelle <arnout at mind.be> wrote:
> On 10/04/14 10:40, Thomas De Schampheleire wrote:
>> Hi,
>>
>> On Fri, Apr 4, 2014 at 8:45 AM, Arnout Vandecappelle <arnout at mind.be> wrote:
>>> On 03/04/14 23:32, Yann E. MORIN wrote:
>>>> Paul, All,
>>>>
>>>> On 2014-04-03 23:01 +0200, Paul Cercueil spake thusly:
> [snip]
>>>> However, this change is still corect, since it makes it explicit that we
>>>> want an unsigned char.
>>>
>>>  But then we should change all the packages that use toupper and
>>> tolower... (and also isdigit and isalnum and ...).
>>>
>>>  Basically, if toupper() asserts in this case, it's a bug in the
>>> toolchain, not in the package.
>>
>> I wouldn't classify it as a bug in the toolchain, but rather as an
>> unfortunate API choice in toupper and friends, or alternatively as an
>> unfortunate failing of the C specification to explicitly define 'char'
>> as being signed or unsigned.
>
>  Yeah, the bane of backward compatibility... Some of these functions
> existed before there were prototypes in C.
>
>
>  Still, I'd say that not allowing a negative signed char value for these
> functions is incorrect. uClibc at least _tries_ to handle it:

The man page of toupper says: (Linux man-pages)

 "If c is not an unsigned char value, or EOF, the behavior of these
functions is undefined."

According to me, the patch (http://patchwork.ozlabs.org/patch/336778/)
is a valid one. The dialog sources should only call toupper with an
unsigned char. There are different ways to fixing this, but the one
proposed in the patch seems fine.

As mentioned before, the statements made by the submitter are correct:
MIPS does indeed have a signed-char by default, it will do
sign-extension on the implicit cast from char to int.

Hence:

Reviewed-by: Thomas De Schampheleire <thomas.de.schampheleire at gmail.com>

Best regards,
Thomas


More information about the buildroot mailing list