Hello,
On Fri, Nov 29, 2024 at 04:13:53PM +0530, Atharva Tiwari wrote:
> - Added a null check for 'o' before copying the last OPTION_END in
> options__order function to prevent potential uninitialized usage.
> - Refactored the parse_long_opt function for improved readability by aligning
> function signature.
> - Minor formatting fix to ensure consistency in the codebase.
> - Updated the wrapper script for pseries architecture to handle 'notext'
> option in a more reliable way.
>
> Signed-off-by: Atharva Tiwari <atharvatiwari1...@outlook.in>
> ---
> arch/powerpc/boot/wrapper | 1 +
> tools/lib/subcmd/parse-options.c | 10 +++++-----
> 2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/boot/wrapper b/arch/powerpc/boot/wrapper
> index 1db60fe13802..d25ad8c622f4 100755
> --- a/arch/powerpc/boot/wrapper
> +++ b/arch/powerpc/boot/wrapper
> @@ -267,6 +267,7 @@ pseries)
> link_address='0x4000000'
> if [ "$format" != "elf32ppc" ]; then
> link_address=
> + notext='-z notext'
This is a completely differnt change and should be sent to powerpc devs
separately.
> pie=-pie
> fi
> make_space=n
> diff --git a/tools/lib/subcmd/parse-options.c
> b/tools/lib/subcmd/parse-options.c
> index 555d617c1f50..f85b642bc9aa 100644
> --- a/tools/lib/subcmd/parse-options.c
> +++ b/tools/lib/subcmd/parse-options.c
> @@ -360,8 +360,7 @@ static int parse_short_opt(struct parse_opt_ctx_t *p,
> const struct option *optio
> return -2;
> }
>
> -static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
> - const struct option *options)
> +static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg, const
> struct option *options)
Please don't change this unnecessarily.
> {
> const char *arg_end = strchr(arg, '=');
> const struct option *abbrev_option = NULL, *ambiguous_option = NULL;
> @@ -828,9 +827,10 @@ static struct option *options__order(const struct option
> *opts)
>
> nr_parent = nr_opts;
> }
> - /* copy the last OPTION_END */
> - memcpy(&ordered[nr_opts], o, sizeof(*o));
> -
> + /* Check whether o is valid before using it to copy the last
> OPTION_END. */
> + if (o && o->type == OPTION_END) {
> + memcpy(&ordered[nr_opts], o, sizeof(*o));
> + }
Do you any real problems with this? I think the existing code assumes
the last entry should be OPTION_END. Otherwise it won't work well.
Thanks,
Namhyung
> /* sort each option group individually */
> for (opt = group = ordered; opt->type != OPTION_END; opt++) {
> if (opt->type == OPTION_GROUP) {
> --
> 2.43.-1
>
