[PATCH] Fix build warnings in locale/programs/ld-ctype.c

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Fix build warnings in locale/programs/ld-ctype.c

Stefan Liebler-2
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.

20190625_warnings_locale_programs_ld-ctype.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix build warnings in locale/programs/ld-ctype.c

Florian Weimer-5
* 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
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix build warnings in locale/programs/ld-ctype.c

Stefan Liebler-2
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
>

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix build warnings in locale/programs/ld-ctype.c

Florian Weimer-5
* 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
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix build warnings in locale/programs/ld-ctype.c

Stefan Liebler-2
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
>

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix build warnings in locale/programs/ld-ctype.c

Florian Weimer-5
* 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
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix build warnings in locale/programs/ld-ctype.c

Stefan Liebler-2
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