Re: [PATCH v2] parse-options: fix xstrdup leak in parse_options_step parse-options:984

From: lidongyan <502024330056@smail.nju.edu.cn>
Date: May 9, 2025, 13:43 UTC
Message-ID: <0F0385E0-E5BB-48E0-8E0E-507AB6857CB8@smail.nju.edu.cn>
In-Reply-To: <ac89ba0a-b50e-469c-99ce-c70b825ae4af@crinan.ddns.net>

On 09/05/2025 21:08，Phillip Wood <phillip.wood123@gmail.com> write：

Show 21 quoted lines

> diff --git a/parse-options.c b/parse-options.c
> index 35fbb3b0d6..9e6e46da27 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -924,12 +924,12 @@ enum parse_opt_result parse_options_step(struct parse_opt_ctx_t *ctx,
>                                         if (internal_help && *ctx->opt == 'h')
>                                                 goto show_usage;
> 
> -                                        /* fake a short option thing to hide the fact that we may have
> +                                        /* move a short option thing to hide the fact that we may have
>                                          * started to parse aggregated stuff
> -                                         *
> -                                         * This is leaky, too bad.
>                                          */
> -                                        ctx->argv[0] = xstrdup(ctx->opt - 1);
> +                                        MOVE_ARRAY((char *)arg, ctx->opt - 1,
> +                                                   strlen(ctx->opt) + 2);
> +                                        ctx->argv[0] = arg;
>                                         *(char *)ctx->argv[0] = '-';
>                                         goto unknown;
>                                 case PARSE_OPT_NON_OPTION:

I’m not sure why git used to use `xtrdup` here instead of modifying `arg` directly. If modifying `arg` is safe, then this is indeed a good solution.