| Message ID | 20141210151334.GA27824@domone |
|---|---|
| State | New |
| Headers | show |
ping On Wed, Dec 10, 2014 at 04:13:34PM +0100, Ondřej Bílka wrote: > On Wed, Dec 10, 2014 at 03:50:53PM +0100, Andreas Schwab wrote: > > Ondřej Bílka <neleai@seznam.cz> writes: > > > > > On Wed, Dec 10, 2014 at 03:09:53PM +0100, Andreas Schwab wrote: > > >> Ondřej Bílka <neleai@seznam.cz> writes: > > >> > > >> > here we return array on stack which is invalid. OK to fix it in obvious way? > > >> > > >> The obvious way would be a static allocation. > > >> > > > which breaks when user modifies array. > > > > Which user? > > > I meant caller, as I looked at code more caller cannot modify it so > static allocation is ok. Also found second occurence of same problem. > > [BZ #17657] > * locale/programs/ld-ctype.c (find_translit2, read_widestring): Return > static array. > > diff --git a/locale/programs/ld-ctype.c b/locale/programs/ld-ctype.c > index 67846b3..eeaf645 100644 > --- a/locale/programs/ld-ctype.c > +++ b/locale/programs/ld-ctype.c > @@ -114,6 +114,9 @@ struct translit_include_t > struct translit_include_t *next; > }; > > +/* Provide some dummy pointer for empty string. */ > +static uint32_t no_str[] = { 0 }; > + > > /* Sparse table of uint32_t. */ > #define TABLE idx_table > @@ -1777,7 +1780,7 @@ find_translit2 (struct locale_ctype_t *ctype, const struct charmap_t *charmap, > > for (wi = tirunp->from; wi <= wch; wi += tirunp->step) > if (wi == wch) > - return (uint32_t []) { 0 }; > + return no_str; > } > } > > @@ -1831,7 +1834,7 @@ read_widestring (struct linereader *ldfile, struct token *now, > > if (now->tok == tok_default_missing) > /* The special name "" will denote this case. */ > - wstr = ((uint32_t *) { 0 }); > + wstr = no_str; > else if (now->tok == tok_bsymbol) > { > /* Get the value from the repertoire. */ > @@ -4090,9 +4093,6 @@ allocate_arrays (struct locale_ctype_t *ctype, const struct charmap_t *charmap, > } > else > { > - /* Provide some dummy pointers since we have nothing to write out. */ > - static uint32_t no_str = { 0 }; > - > ctype->translit_from_idx = &no_str; > ctype->translit_from_tbl = &no_str; > ctype->translit_to_tbl = &no_str;
Ok. Andreas.
On Wed, 2014-12-10 at 16:13 +0100, Ondřej Bílka wrote: > diff --git a/locale/programs/ld-ctype.c b/locale/programs/ld-ctype.c > index 67846b3..eeaf645 100644 > --- a/locale/programs/ld-ctype.c > +++ b/locale/programs/ld-ctype.c > @@ -114,6 +114,9 @@ struct translit_include_t > struct translit_include_t *next; > }; > > +/* Provide some dummy pointer for empty string. */ > +static uint32_t no_str[] = { 0 }; > + > > /* Sparse table of uint32_t. */ > #define TABLE idx_table [...] > @@ -4090,9 +4093,6 @@ allocate_arrays (struct locale_ctype_t *ctype, const struct charmap_t *charmap, > } > else > { > - /* Provide some dummy pointers since we have nothing to write out. */ > - static uint32_t no_str = { 0 }; > - > ctype->translit_from_idx = &no_str; > ctype->translit_from_tbl = &no_str; > ctype->translit_to_tbl = &no_str; This 'no_str' and the one you introduced above are different. You need to drop the &'s here. Did you build and test with warnings enabled? A fairly recent GCC complains about the different pointer types.
On Mon, Dec 15, 2014 at 04:03:20PM +0100, Torvald Riegel wrote: > On Wed, 2014-12-10 at 16:13 +0100, Ondřej Bílka wrote: > > diff --git a/locale/programs/ld-ctype.c b/locale/programs/ld-ctype.c > > index 67846b3..eeaf645 100644 > > --- a/locale/programs/ld-ctype.c > > +++ b/locale/programs/ld-ctype.c > > @@ -114,6 +114,9 @@ struct translit_include_t > > struct translit_include_t *next; > > }; > > > > +/* Provide some dummy pointer for empty string. */ > > +static uint32_t no_str[] = { 0 }; > > + > > > > /* Sparse table of uint32_t. */ > > #define TABLE idx_table > > [...] > > > @@ -4090,9 +4093,6 @@ allocate_arrays (struct locale_ctype_t *ctype, const struct charmap_t *charmap, > > } > > else > > { > > - /* Provide some dummy pointers since we have nothing to write out. */ > > - static uint32_t no_str = { 0 }; > > - > > ctype->translit_from_idx = &no_str; > > ctype->translit_from_tbl = &no_str; > > ctype->translit_to_tbl = &no_str; > > This 'no_str' and the one you introduced above are different. You need > to drop the &'s here. Did you build and test with warnings enabled? A > fairly recent GCC complains about the different pointer types. Yes, I run test in parallel with writing mail and here I fixed that in test but forgot to modify mail.
diff --git a/locale/programs/ld-ctype.c b/locale/programs/ld-ctype.c index 67846b3..eeaf645 100644 --- a/locale/programs/ld-ctype.c +++ b/locale/programs/ld-ctype.c @@ -114,6 +114,9 @@ struct translit_include_t struct translit_include_t *next; }; +/* Provide some dummy pointer for empty string. */ +static uint32_t no_str[] = { 0 }; + /* Sparse table of uint32_t. */ #define TABLE idx_table @@ -1777,7 +1780,7 @@ find_translit2 (struct locale_ctype_t *ctype, const struct charmap_t *charmap, for (wi = tirunp->from; wi <= wch; wi += tirunp->step) if (wi == wch) - return (uint32_t []) { 0 }; + return no_str; } } @@ -1831,7 +1834,7 @@ read_widestring (struct linereader *ldfile, struct token *now, if (now->tok == tok_default_missing) /* The special name "" will denote this case. */ - wstr = ((uint32_t *) { 0 }); + wstr = no_str; else if (now->tok == tok_bsymbol) { /* Get the value from the repertoire. */ @@ -4090,9 +4093,6 @@ allocate_arrays (struct locale_ctype_t *ctype, const struct charmap_t *charmap, } else { - /* Provide some dummy pointers since we have nothing to write out. */ - static uint32_t no_str = { 0 }; - ctype->translit_from_idx = &no_str; ctype->translit_from_tbl = &no_str; ctype->translit_to_tbl = &no_str;