[BZ,#17657] Return allocated array instead array on stack.
Commit Message
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.
Comments
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;
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.
@@ -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;