Message ID | 20230713124813.216028-1-josimmon@redhat.com |
---|---|
State | New |
Headers | show |
Series | argp-parse: Get rid of alloca | expand |
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 >
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 > > >
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 > > > > > >
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 --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. */