Thomas Huth <th...@redhat.com> writes:
> On 18/04/2019 16.53, Markus Armbruster wrote:
>> atoui() and get_index() pass char values to isdigit(). With a
>> standard isdigit(), we'd get undefined behavior when the value is
>> negative. But we're using isdigit() from pc-bios/s390-ccw/libc.h
>> here, which behaves nicely. Clean up anyway, just to avoid setting a
>> bad example.
>>
>> Cc: Christian Borntraeger <borntrae...@de.ibm.com>
>> Cc: Thomas Huth <th...@redhat.com>
>> Cc: Cornelia Huck <coh...@redhat.com>
>> Cc: qemu-s3...@nongnu.org
>> Signed-off-by: Markus Armbruster <arm...@redhat.com>
>> ---
>> pc-bios/s390-ccw/libc.c | 2 +-
>> pc-bios/s390-ccw/menu.c | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/pc-bios/s390-ccw/libc.c b/pc-bios/s390-ccw/libc.c
>> index a786566c4c..3187923950 100644
>> --- a/pc-bios/s390-ccw/libc.c
>> +++ b/pc-bios/s390-ccw/libc.c
>> @@ -38,7 +38,7 @@ uint64_t atoui(const char *str)
>> }
>>
>> while (*str) {
>> - if (!isdigit(*str)) {
>> + if (!isdigit(*(unsigned char *)str)) {
>> break;
>> }
>> val = val * 10 + *str - '0';
>> diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c
>> index 82a4ae6315..ce3815b201 100644
>> --- a/pc-bios/s390-ccw/menu.c
>> +++ b/pc-bios/s390-ccw/menu.c
>> @@ -134,7 +134,7 @@ static int get_index(void)
>>
>> /* Check for erroneous input */
>> for (i = 0; i < len; i++) {
>> - if (!isdigit(buf[i])) {
>> + if (!isdigit((unsigned char)buf[i])) {
>> return -1;
>> }
>> }
>
> FWIW, "char" is unsigned by default on s390x, so this is doing nothing.
I see.
If we decide to keep the patch, the commit message needs tweaking.
Perhaps:
atoui() and get_index() pass char values to isdigit(). With a
standard isdigit(), we'd get undefined behavior when the value is
negative. Can't happen as char is unsigned on s390x. Even if it
could, we're actually using isdigit() from pc-bios/s390-ccw/libc.h
here, which works fine for negative values. Clean up anyway, just
to avoid setting a bad example.
