| Message ID | 45faf360-cad3-7c9b-a914-0823d2724b90@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix build warnings in locale/programs/ld-ctype.c | expand |
* 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.
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;
if (seq != NULL && seq->nbytes == 1)
/* Yep, we can store information about this byte sequence. */
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.