Thread (2 messages) flat view 2 messages, 2 authors, 2025-11-25

Re: `git config get --type=path` results in segmentation fault on value starting with `:(optional)`

From: Junio C Hamano <hidden>
Date: 2025-11-20 16:46:21

Possibly related (same subject, not in this thread)

Jeff King [off-list ref] writes:

Thanks for analysing all of the above (omitted); I was doing the
same on the bus but couldn't finish it and then when I reached the
office, you've nicely done everything necessary ;-)
I put a [*] above on "more or less does the right thing" because there's
another corner case, even for callers like commit.template. What should
this:

  [commit]
  template = :(optional)does-exist
  template = :(optional)does-not-exist

With the current code, we will ignore the second config entry entirely,
and the result will point to "does-exist". But that feels surprising to
me.
The documentation says

	If prefixed with :(optional), the configuration variable is
	treated as if it does not exist, if the named path does not
	exist.

and when I wrote it, by "the configuration variable", I meant the
second "template = ..." line above, not the configuration variable
commit.template, that the machinery pretends not to exist.  So the
result pointing at does-exist matches my expectation.
I kind of wonder if git_config_pathname() ought to be returning more
data to the caller, like:

  struct config_pathname {
	char *path; /* never NULL */
	unsigned missing : 1;
  };

That would change the interface of git_config_pathname(), but that would
also force us to make the appropriate changes in each caller.
The problem is that there is no mechanism for the function to say
"success" without setting *dest to the discovered value.  We could
introduce multiple kinds of "failure", and have callers react to the
differences, but then it is like setting NULL in *dest and having
callers react to it, so I am not sure how much benefit we would be
gaining by changing its interface.

On the other hand, builtin/config.c::format_config() probably needs
a richer set of return values.  When used from collect_config(), it
needs to be able to say "no, pretend that the key/value pair you fed
me did not exist" in addition to "that value is bogus---you have an
error (e.g., config_error_nonbool())".
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help