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

From: Phillip Wood <phillip.wood123@gmail.com>
Date: May 9, 2025, 13:08 UTC
Message-ID: <ac89ba0a-b50e-469c-99ce-c70b825ae4af@crinan.ddns.net>
In-Reply-To: <aB2ejA1tCK9DR1Nq@pks.im>

On 09/05/2025 07:19, Patrick Steinhardt wrote:

> 
> All of that starts to become kind of involved though. So unless others
> disagree with my analysis I just don't think this edge case really is
> worth the complexity.

I agree with this. We seem to be copying the string because argv is a const char** but would it really be so bad to modify the string that gets passed to us? We know that any bundled options that come before an unknown option cannot have a value or else what we see as the unknown option would be part of that value. So I think we could do

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:

Best Wishes

Phillip