Re: [PATCH] config.c: change the function signature of `git_config_string()`

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> So I would not mind lifting this unnecessary restriction on
> git_config_string, but I do not see a way to do it without making the
> rest of the code much uglier (and I do not see a particular advantage in
> modifying git_config_string here that would make it worth the trouble).

I do not think changing the existing one is warranted, either.

Given that C's type system does not allow us to pass "const char *"
as "char *" without cast (and vice versa), a function that takes an
out parameter that points at the location to store the pointer to a
string (instead of returning the pointer to a string as value) has
to take either one of these forms:

	get_mutable(char **dest);
        get_const(const char **dest);

and half the callers need to pass their variables with a cast, i.e.

        char *mutable_string;
	const char *const_string;

	get_mutable((char **)&const_string);
        get_mutable(&mutable_string);
	get_const(&const_string);
	get_const((const char **)&mutable_string);

If we have to cast some [*1*], I'd prefer to see the type describe
what it really is, and I think a function that gives a newly
allocated storage for the callers' own use is better described by
taking a pointer to non-const "char *" location.  So I'd encourage a
new function being introduced that does not have existing callers to
use that as the criterion to decide which to take.

Changing the existing function with existing calling site needs two
other pros-and-cons evaluation, in addition to the above "does the
type describe what it really is?", which are "is it worth the
churn?" and "does the end result make more sense than the original?"


[Footnote]

*1* We have safe_create_leading_directories_const() that works
around this for input parameter around its _const less counterpart,
which is ugly but livable solution.