Fix build warnings in locale/programs/ld-ctype.c
Commit Message
Hi,
this patch fixes the gcc warnings seen with gcc 9 -march>=z13 on s390x:
programs/ld-ctype.c: In function ‘ctype_read’:
programs/ld-ctype.c:1392:13: error: ‘wch’ may be used uninitialized in
this function [-Werror=maybe-uninitialized]
1392 | uint32_t wch;
| ^~~
programs/ld-ctype.c:1401:7: error: ‘seq’ may be used uninitialized in
this function [-Werror=maybe-uninitialized]
1401 | if (seq != NULL && seq->nbytes == 1)
| ^
programs/ld-ctype.c:1391:20: note: ‘seq’ was declared here
1391 | struct charseq *seq;
| ^~~
Both seq and wch are uninitialized if get_character fails.
Thus we are now returning with an error.
Bye
Stefan
ChangeLog:
* locale/programs/ld-ctype.c (charclass_symbolic_ellipsis):
Return error if get_character fails.
Comments
* Stefan Liebler:
> diff --git a/locale/programs/ld-ctype.c b/locale/programs/ld-ctype.c
> index e6105928da..cfc9c43fd5 100644
> --- a/locale/programs/ld-ctype.c
> +++ b/locale/programs/ld-ctype.c
> @@ -1396,7 +1396,8 @@ charclass_symbolic_ellipsis (struct linereader *ldfile,
> (int) (now->val.str.lenmb - (cp - last_str)),
> from);
>
> - get_character (now, charmap, repertoire, &seq, &wch);
> + if (get_character (now, charmap, repertoire, &seq, &wch))
> + goto invalid_range;
Maybe write:
if (get_character (now, charmap, repertoire, &seq, &wch) != 0)
to match the other function calls?
Otherwise, looks good.
Thanks,
Florian
On 6/25/19 3:23 PM, Florian Weimer wrote:
> * Stefan Liebler:
>
>> diff --git a/locale/programs/ld-ctype.c b/locale/programs/ld-ctype.c
>> index e6105928da..cfc9c43fd5 100644
>> --- a/locale/programs/ld-ctype.c
>> +++ b/locale/programs/ld-ctype.c
>> @@ -1396,7 +1396,8 @@ charclass_symbolic_ellipsis (struct linereader *ldfile,
>> (int) (now->val.str.lenmb - (cp - last_str)),
>> from);
>>
>> - get_character (now, charmap, repertoire, &seq, &wch);
>> + if (get_character (now, charmap, repertoire, &seq, &wch))
>> + goto invalid_range;
>
> Maybe write:
>
> if (get_character (now, charmap, repertoire, &seq, &wch) != 0)
>
> to match the other function calls?
Okay. That's no problem. If no one opposes, I'll commit the patch
tomorrow with "!= 0".
Shall I also update the following occurrence in ctype_read?
if (ellipsis_token == tok_none)
{
if (get_character (now, charmap, repertoire, &seq, &wch))
goto err_label;
>
> Otherwise, looks good.
>
> Thanks,
> Florian
>
* Stefan Liebler:
> On 6/25/19 3:23 PM, Florian Weimer wrote:
>> * Stefan Liebler:
>>
>>> diff --git a/locale/programs/ld-ctype.c b/locale/programs/ld-ctype.c
>>> index e6105928da..cfc9c43fd5 100644
>>> --- a/locale/programs/ld-ctype.c
>>> +++ b/locale/programs/ld-ctype.c
>>> @@ -1396,7 +1396,8 @@ charclass_symbolic_ellipsis (struct linereader *ldfile,
>>> (int) (now->val.str.lenmb - (cp - last_str)),
>>> from);
>>> - get_character (now, charmap, repertoire, &seq, &wch);
>>> + if (get_character (now, charmap, repertoire, &seq, &wch))
>>> + goto invalid_range;
>>
>> Maybe write:
>>
>> if (get_character (now, charmap, repertoire, &seq, &wch) != 0)
>>
>> to match the other function calls?
> Okay. That's no problem. If no one opposes, I'll commit the patch
> tomorrow with "!= 0".
>
> Shall I also update the following occurrence in ctype_read?
> if (ellipsis_token == tok_none)
> {
> if (get_character (now, charmap, repertoire, &seq, &wch))
> goto err_label;
Oh, I had missed that. If the calls are already inconsistent, you can
use your original patch, too.
To be honest, I'm more concerned about the other calls to get_character
without error checking.
Thanks,
Florian
On 6/25/19 3:39 PM, Florian Weimer wrote:
> * Stefan Liebler:
>
>> On 6/25/19 3:23 PM, Florian Weimer wrote:
>>> * Stefan Liebler:
>>>
>>>> diff --git a/locale/programs/ld-ctype.c b/locale/programs/ld-ctype.c
>>>> index e6105928da..cfc9c43fd5 100644
>>>> --- a/locale/programs/ld-ctype.c
>>>> +++ b/locale/programs/ld-ctype.c
>>>> @@ -1396,7 +1396,8 @@ charclass_symbolic_ellipsis (struct linereader *ldfile,
>>>> (int) (now->val.str.lenmb - (cp - last_str)),
>>>> from);
>>>> - get_character (now, charmap, repertoire, &seq, &wch);
>>>> + if (get_character (now, charmap, repertoire, &seq, &wch))
>>>> + goto invalid_range;
>>>
>>> Maybe write:
>>>
>>> if (get_character (now, charmap, repertoire, &seq, &wch) != 0)
>>>
>>> to match the other function calls?
>> Okay. That's no problem. If no one opposes, I'll commit the patch
>> tomorrow with "!= 0".
>>
>> Shall I also update the following occurrence in ctype_read?
>> if (ellipsis_token == tok_none)
>> {
>> if (get_character (now, charmap, repertoire, &seq, &wch))
>> goto err_label;
>
> Oh, I had missed that. If the calls are already inconsistent, you can
> use your original patch, too.
Okay. Then I'll use the original patch.
>
> To be honest, I'm more concerned about the other calls to get_character
> without error checking.
Where do you see other ones?
With my patch applied, I just see the following occurrences of
get_character which are now all checking the return value:
1257:get_character (struct token *now, const struct charmap_t *charmap,
1399:if (get_character (now, charmap, repertoire, &seq, &wch))
2291:if (get_character (now, charmap, repertoire, &seq, &wch))
2574:if (get_character (now, charmap, repertoire, &from_seq,
2585:if (get_character (now, charmap, repertoire, &to_seq,
>
> Thanks,
> Florian
>
* Stefan Liebler:
>> To be honest, I'm more concerned about the other calls to get_character
>> without error checking.
> Where do you see other ones?
> With my patch applied, I just see the following occurrences of
> get_character which are now all checking the return value:
> 1257:get_character (struct token *now, const struct charmap_t *charmap,
> 1399:if (get_character (now, charmap, repertoire, &seq, &wch))
> 2291:if (get_character (now, charmap, repertoire, &seq, &wch))
> 2574:if (get_character (now, charmap, repertoire, &from_seq,
> 2585:if (get_character (now, charmap, repertoire, &to_seq,
Ah, my bad, you are right. Please commit the original patch.
Thanks,
Florian
On 6/25/19 3:54 PM, Florian Weimer wrote:
> * Stefan Liebler:
>
>>> To be honest, I'm more concerned about the other calls to get_character
>>> without error checking.
>
>> Where do you see other ones?
>> With my patch applied, I just see the following occurrences of
>> get_character which are now all checking the return value:
>> 1257:get_character (struct token *now, const struct charmap_t *charmap,
>> 1399:if (get_character (now, charmap, repertoire, &seq, &wch))
>> 2291:if (get_character (now, charmap, repertoire, &seq, &wch))
>> 2574:if (get_character (now, charmap, repertoire, &from_seq,
>> 2585:if (get_character (now, charmap, repertoire, &to_seq,
>
> Ah, my bad, you are right. Please commit the original patch.
>
> Thanks,
> Florian
>
Committed.
Thanks
commit 0dca38c4afd868da65e0b5c0d76fc30bd3114994
Author: Stefan Liebler <stli@linux.ibm.com>
Date: Tue Jun 25 08:57:40 2019 +0200
Fix build warnings in locale/programs/ld-ctype.c
This patch fixes the gcc warnings seen with gcc 9 -march>=z13 on s390x:
programs/ld-ctype.c: In function ‘ctype_read’:
programs/ld-ctype.c:1392:13: error: ‘wch’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
1392 | uint32_t wch;
| ^~~
programs/ld-ctype.c:1401:7: error: ‘seq’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
1401 | if (seq != NULL && seq->nbytes == 1)
| ^
programs/ld-ctype.c:1391:20: note: ‘seq’ was declared here
1391 | struct charseq *seq;
| ^~~
Both seq and wch are uninitialized if get_character fails.
Thus we are now returning with an error.
ChangeLog:
* locale/programs/ld-ctype.c (charclass_symbolic_ellipsis):
Return error if get_character fails.
@@ -1396,7 +1396,8 @@ charclass_symbolic_ellipsis (struct linereader *ldfile,
(int) (now->val.str.lenmb - (cp - last_str)),
from);
- get_character (now, charmap, repertoire, &seq, &wch);
+ if (get_character (now, charmap, repertoire, &seq, &wch))
+ goto invalid_range;
if (seq != NULL && seq->nbytes == 1)
/* Yep, we can store information about this byte sequence. */