[BZ,#17657] Return allocated array instead array on stack.
diff mbox

Message ID 20141210151334.GA27824@domone
State New
Headers show

Commit Message

Ondřej Bílka Dec. 10, 2014, 3:13 p.m. UTC
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

Ondřej Bílka Dec. 12, 2014, 4:26 p.m. UTC | #1
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;
Andreas Schwab Dec. 15, 2014, 9:05 a.m. UTC | #2
Ok.

Andreas.
Torvald Riegel Dec. 15, 2014, 3:03 p.m. UTC | #3
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.
Ondřej Bílka Dec. 15, 2014, 10:37 p.m. UTC | #4
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.

Patch
diff mbox

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;