Re: [PATCH 02/18] config: Introduce functions to write non-standard file

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jul 19, 2011, 20:35 UTC
Message-ID: <20110719203541.GB26557@elie.gateway.2wire.net>
In-Reply-To: <20110719195504.GA3957@sigill.intra.peff.net>

Jeff King wrote:

Show 15 quoted lines

> BTW, I'm unclear why we bother duplicating the filename in the first
> place. It seems like we could go even simpler with:
>
> int git_config_set_multivar(const char *key, const char *value,
>                             const char *value_regex, int multi_replace)
> {
> 	char *config_filename;
>
> 	if (config_exclusive_filename)
> 		config_filename = config_exclusive_filename;
> 	else
> 		config_filename = git_path("config");
>
>         return git_config_set_multivar_in_file(config_filename, key, value,
>                                                value_regex, multi_replace);

FWIW, I suspect that would be a trap waiting to be triggered as soon as git_config_set_multivar_in_file() calls git_path() four times. I.e., it's not about the config_exclusive_filename but about the git_path.

A little micro-optimization might be to do the following. :)

	const char *config_filename;
	char *filename_buf = NULL;
	int result;

	if (config_exclusive_filename)
		config_filename = config_exclusive_filename;
	else
		config_filename = filename_buf = git_pathdup("config");

	result = git_config_set_multivar_in_file(...);
	free(filename_buf);
	return result;