Re: [BUG] revision: premature free-and-null causes “unknown option `(null)`”

[Jeff King <peff@peff.net>]

On Fri, Sep 25, 2026 at 02:28:57AM +0200, Kristoffer Haugsbakk wrote:

> git shortlog -n --not-an-option master
> [...]
>
> I expected it to print the option in quotes. Instead it printed `(null)`
> which I think is the placeholder for when the `%s` arg is `NULL`.
> [...]
> 
> I have bisected this to cd439487 (revision: manage memory ownership of
> argv in setup_revisions(), 2025-09-19).

Yep, definitely my fault. I don't have time to do a full write-up now,
but the most direct solution is:

diff --git a/revision.c b/revision.c
index ee1df92d1d..501a4ba36e 100644
--- a/revision.c
+++ b/revision.c
@@ -2768,13 +2768,13 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,
 			const struct option *options,
 			const char * const usagestr[])
 {
 	int n = handle_revision_opt(revs, ctx->argc, ctx->argv,
 				    &ctx->cpidx, ctx->out, NULL);
 	if (n <= 0) {
-		error("unknown option `%s'", ctx->argv[0]);
+		error("unknown option `%s'", ctx->out[ctx->cpidx - 1]);
 		usage_with_options(usagestr, options);
 	}
 	ctx->argv += n;
 	ctx->argc -= n;
 }

But I think instead doing this:

diff --git a/revision.c b/revision.c
index ee1df92d1d..7b858d54c1 100644
--- a/revision.c
+++ b/revision.c
@@ -2340,7 +2340,8 @@ static void overwrite_argv(int *argc, const char **argv,
 	if (*value != argv[*argc]) {
 		mark_argv_for_free(opt, revs, argv[*argc]);
 		argv[*argc] = *value;
-		*value = NULL;
+		if (opt && opt->free_removed_argv_elements)
+			*value = NULL;
 	}
 	(*argc)++;
 }

will restore some of the hidden assumptions made by pre-cd439487 code.
So it would fix this case, along with any other lurkers.

I know that's probably quite opaque. ;) I'll fully explain what's going
on in a follow-up tomorrow, but I wanted to post the solution quickly so
nobody else wasted time digging.

Thanks for a clear report.

-Peff