diff mbox series

argp-parse: Get rid of alloca

Message ID 20230713124813.216028-1-josimmon@redhat.com
State Superseded
Headers
Series argp-parse: Get rid of alloca |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
redhat-pt-bot/TryBot-32bit success Build for i686
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 fail Testing failed
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Testing passed

Commit Message

Joe Simmons-Talbott July 13, 2023, 12:48 p.m. UTC 
  Even though the alloca usage is relatively small and fixed size the code
can be written without using alloca.  Convert to local variables.

Checked on x86_64-linux-gnu.
---
 argp/argp-parse.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

Comments

Joe Simmons-Talbott Aug. 3, 2023, 1:10 p.m. UTC | #1 
On Thu, Jul 13, 2023 at 08:48:13AM -0400, Joe Simmons-Talbott wrote:
> Even though the alloca usage is relatively small and fixed size the code
> can be written without using alloca.  Convert to local variables.

Ping.

Thanks,
Joe
> 
> Checked on x86_64-linux-gnu.
> ---
>  argp/argp-parse.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/argp/argp-parse.c b/argp/argp-parse.c
> index a44b50f8e4..40a5896d21 100644
> --- a/argp/argp-parse.c
> +++ b/argp/argp-parse.c
> @@ -884,6 +884,9 @@ __argp_parse (const struct argp *argp, int argc, char **argv, unsigned flags,
>    error_t err;
>    struct parser parser;
>  
> +  struct argp_child child[4];
> +  struct argp top_argp;
> +
>    /* If true, then err == EBADKEY is a result of a non-option argument failing
>       to be parsed (which in some cases isn't actually an error).  */
>    int arg_ebadkey = 0;
> @@ -891,24 +894,23 @@ __argp_parse (const struct argp *argp, int argc, char **argv, unsigned flags,
>    if (! (flags & ARGP_NO_HELP))
>      /* Add our own options.  */
>      {
> -      struct argp_child *child = alloca (4 * sizeof (struct argp_child));
> -      struct argp *top_argp = alloca (sizeof (struct argp));
> +      int child_index = 0;
>  
>        /* TOP_ARGP has no options, it just serves to group the user & default
>  	 argps.  */
> -      memset (top_argp, 0, sizeof (*top_argp));
> -      top_argp->children = child;
> +      memset (&top_argp, 0, sizeof (struct argp));
> +      top_argp.children = child;
>  
>        memset (child, 0, 4 * sizeof (struct argp_child));
>  
>        if (argp)
> -	(child++)->argp = argp;
> -      (child++)->argp = &argp_default_argp;
> +	child[child_index++].argp = argp;
> +      child[child_index++].argp = &argp_default_argp;
>        if (argp_program_version || argp_program_version_hook)
> -	(child++)->argp = &argp_version_argp;
> -      child->argp = 0;
> +	child[child_index++].argp = &argp_version_argp;
> +      child[child_index].argp = 0;
>  
> -      argp = top_argp;
> +      argp = &top_argp;
>      }
>  
>    /* Construct a parser for these arguments.  */
> -- 
> 2.39.2
>
Joe Simmons-Talbott Aug. 15, 2023, 2:49 p.m. UTC | #2 
On Thu, Aug 03, 2023 at 09:10:49AM -0400, Joe Simmons-Talbott via Libc-alpha wrote:
> On Thu, Jul 13, 2023 at 08:48:13AM -0400, Joe Simmons-Talbott wrote:
> > Even though the alloca usage is relatively small and fixed size the code
> > can be written without using alloca.  Convert to local variables.
> 
> Ping.
Ping.

Thanks,
Joe
> 
> Thanks,
> Joe
> > 
> > Checked on x86_64-linux-gnu.
> > ---
> >  argp/argp-parse.c | 20 +++++++++++---------
> >  1 file changed, 11 insertions(+), 9 deletions(-)
> > 
> > diff --git a/argp/argp-parse.c b/argp/argp-parse.c
> > index a44b50f8e4..40a5896d21 100644
> > --- a/argp/argp-parse.c
> > +++ b/argp/argp-parse.c
> > @@ -884,6 +884,9 @@ __argp_parse (const struct argp *argp, int argc, char **argv, unsigned flags,
> >    error_t err;
> >    struct parser parser;
> >  
> > +  struct argp_child child[4];
> > +  struct argp top_argp;
> > +
> >    /* If true, then err == EBADKEY is a result of a non-option argument failing
> >       to be parsed (which in some cases isn't actually an error).  */
> >    int arg_ebadkey = 0;
> > @@ -891,24 +894,23 @@ __argp_parse (const struct argp *argp, int argc, char **argv, unsigned flags,
> >    if (! (flags & ARGP_NO_HELP))
> >      /* Add our own options.  */
> >      {
> > -      struct argp_child *child = alloca (4 * sizeof (struct argp_child));
> > -      struct argp *top_argp = alloca (sizeof (struct argp));
> > +      int child_index = 0;
> >  
> >        /* TOP_ARGP has no options, it just serves to group the user & default
> >  	 argps.  */
> > -      memset (top_argp, 0, sizeof (*top_argp));
> > -      top_argp->children = child;
> > +      memset (&top_argp, 0, sizeof (struct argp));
> > +      top_argp.children = child;
> >  
> >        memset (child, 0, 4 * sizeof (struct argp_child));
> >  
> >        if (argp)
> > -	(child++)->argp = argp;
> > -      (child++)->argp = &argp_default_argp;
> > +	child[child_index++].argp = argp;
> > +      child[child_index++].argp = &argp_default_argp;
> >        if (argp_program_version || argp_program_version_hook)
> > -	(child++)->argp = &argp_version_argp;
> > -      child->argp = 0;
> > +	child[child_index++].argp = &argp_version_argp;
> > +      child[child_index].argp = 0;
> >  
> > -      argp = top_argp;
> > +      argp = &top_argp;
> >      }
> >  
> >    /* Construct a parser for these arguments.  */
> > -- 
> > 2.39.2
> > 
>
Joe Simmons-Talbott Aug. 28, 2023, 1:20 p.m. UTC | #3 
Ping.

On Tue, Aug 15, 2023 at 10:49:12AM -0400, Joe Simmons-Talbott via Libc-alpha wrote:
> On Thu, Aug 03, 2023 at 09:10:49AM -0400, Joe Simmons-Talbott via Libc-alpha wrote:
> > On Thu, Jul 13, 2023 at 08:48:13AM -0400, Joe Simmons-Talbott wrote:
> > > Even though the alloca usage is relatively small and fixed size the code
> > > can be written without using alloca.  Convert to local variables.
> > 
> > Ping.
> Ping.
> 
> Thanks,
> Joe
> > 
> > Thanks,
> > Joe
> > > 
> > > Checked on x86_64-linux-gnu.
> > > ---
> > >  argp/argp-parse.c | 20 +++++++++++---------
> > >  1 file changed, 11 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/argp/argp-parse.c b/argp/argp-parse.c
> > > index a44b50f8e4..40a5896d21 100644
> > > --- a/argp/argp-parse.c
> > > +++ b/argp/argp-parse.c
> > > @@ -884,6 +884,9 @@ __argp_parse (const struct argp *argp, int argc, char **argv, unsigned flags,
> > >    error_t err;
> > >    struct parser parser;
> > >  
> > > +  struct argp_child child[4];
> > > +  struct argp top_argp;
> > > +
> > >    /* If true, then err == EBADKEY is a result of a non-option argument failing
> > >       to be parsed (which in some cases isn't actually an error).  */
> > >    int arg_ebadkey = 0;
> > > @@ -891,24 +894,23 @@ __argp_parse (const struct argp *argp, int argc, char **argv, unsigned flags,
> > >    if (! (flags & ARGP_NO_HELP))
> > >      /* Add our own options.  */
> > >      {
> > > -      struct argp_child *child = alloca (4 * sizeof (struct argp_child));
> > > -      struct argp *top_argp = alloca (sizeof (struct argp));
> > > +      int child_index = 0;
> > >  
> > >        /* TOP_ARGP has no options, it just serves to group the user & default
> > >  	 argps.  */
> > > -      memset (top_argp, 0, sizeof (*top_argp));
> > > -      top_argp->children = child;
> > > +      memset (&top_argp, 0, sizeof (struct argp));
> > > +      top_argp.children = child;
> > >  
> > >        memset (child, 0, 4 * sizeof (struct argp_child));
> > >  
> > >        if (argp)
> > > -	(child++)->argp = argp;
> > > -      (child++)->argp = &argp_default_argp;
> > > +	child[child_index++].argp = argp;
> > > +      child[child_index++].argp = &argp_default_argp;
> > >        if (argp_program_version || argp_program_version_hook)
> > > -	(child++)->argp = &argp_version_argp;
> > > -      child->argp = 0;
> > > +	child[child_index++].argp = &argp_version_argp;
> > > +      child[child_index].argp = 0;
> > >  
> > > -      argp = top_argp;
> > > +      argp = &top_argp;
> > >      }
> > >  
> > >    /* Construct a parser for these arguments.  */
> > > -- 
> > > 2.39.2
> > > 
> > 
>
Adhemerval Zanella Netto Aug. 28, 2023, 4:44 p.m. UTC | #4 
On 13/07/23 09:48, Joe Simmons-Talbott via Libc-alpha wrote:
> Even though the alloca usage is relatively small and fixed size the code
> can be written without using alloca.  Convert to local variables.
> 
> Checked on x86_64-linux-gnu.

You can also remove all the boilerplate that define alloca at the start.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  argp/argp-parse.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/argp/argp-parse.c b/argp/argp-parse.c
> index a44b50f8e4..40a5896d21 100644
> --- a/argp/argp-parse.c
> +++ b/argp/argp-parse.c
> @@ -884,6 +884,9 @@ __argp_parse (const struct argp *argp, int argc, char **argv, unsigned flags,
>    error_t err;
>    struct parser parser;
>  
> +  struct argp_child child[4];
> +  struct argp top_argp;
> +
>    /* If true, then err == EBADKEY is a result of a non-option argument failing
>       to be parsed (which in some cases isn't actually an error).  */
>    int arg_ebadkey = 0;
> @@ -891,24 +894,23 @@ __argp_parse (const struct argp *argp, int argc, char **argv, unsigned flags,
>    if (! (flags & ARGP_NO_HELP))
>      /* Add our own options.  */
>      {
> -      struct argp_child *child = alloca (4 * sizeof (struct argp_child));
> -      struct argp *top_argp = alloca (sizeof (struct argp));
> +      int child_index = 0;
>  
>        /* TOP_ARGP has no options, it just serves to group the user & default
>  	 argps.  */
> -      memset (top_argp, 0, sizeof (*top_argp));
> -      top_argp->children = child;
> +      memset (&top_argp, 0, sizeof (struct argp));
> +      top_argp.children = child;
>  
>        memset (child, 0, 4 * sizeof (struct argp_child));
>  
>        if (argp)
> -	(child++)->argp = argp;
> -      (child++)->argp = &argp_default_argp;
> +	child[child_index++].argp = argp;
> +      child[child_index++].argp = &argp_default_argp;
>        if (argp_program_version || argp_program_version_hook)
> -	(child++)->argp = &argp_version_argp;
> -      child->argp = 0;
> +	child[child_index++].argp = &argp_version_argp;
> +      child[child_index].argp = 0;
>  
> -      argp = top_argp;
> +      argp = &top_argp;
>      }
>  
>    /* Construct a parser for these arguments.  */
diff mbox series

Patch

diff --git a/argp/argp-parse.c b/argp/argp-parse.c
index a44b50f8e4..40a5896d21 100644
--- a/argp/argp-parse.c
+++ b/argp/argp-parse.c
@@ -884,6 +884,9 @@  __argp_parse (const struct argp *argp, int argc, char **argv, unsigned flags,
   error_t err;
   struct parser parser;
 
+  struct argp_child child[4];
+  struct argp top_argp;
+
   /* If true, then err == EBADKEY is a result of a non-option argument failing
      to be parsed (which in some cases isn't actually an error).  */
   int arg_ebadkey = 0;
@@ -891,24 +894,23 @@  __argp_parse (const struct argp *argp, int argc, char **argv, unsigned flags,
   if (! (flags & ARGP_NO_HELP))
     /* Add our own options.  */
     {
-      struct argp_child *child = alloca (4 * sizeof (struct argp_child));
-      struct argp *top_argp = alloca (sizeof (struct argp));
+      int child_index = 0;
 
       /* TOP_ARGP has no options, it just serves to group the user & default
 	 argps.  */
-      memset (top_argp, 0, sizeof (*top_argp));
-      top_argp->children = child;
+      memset (&top_argp, 0, sizeof (struct argp));
+      top_argp.children = child;
 
       memset (child, 0, 4 * sizeof (struct argp_child));
 
       if (argp)
-	(child++)->argp = argp;
-      (child++)->argp = &argp_default_argp;
+	child[child_index++].argp = argp;
+      child[child_index++].argp = &argp_default_argp;
       if (argp_program_version || argp_program_version_hook)
-	(child++)->argp = &argp_version_argp;
-      child->argp = 0;
+	child[child_index++].argp = &argp_version_argp;
+      child[child_index].argp = 0;
 
-      argp = top_argp;
+      argp = &top_argp;
     }
 
   /* Construct a parser for these arguments.  */