Re: [PATCH] environment.c: introduce SETUP_GIT_ENV helper macro

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 27, 2016, 18:50 UTC
Message-ID: <xmqqsi0ef0mz.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <xmqqpovighxk.fsf@gitster.mtv.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 25 quoted lines

> Alexander Kuleshov <kuleshovmail@gmail.com> writes:
>
>> Let's move this to the SETUP_GIT_ENV helper macro to prevent
>> code duplication in these functions.
>
> Please don't.  A macro that hides "return" makes things harder to
> follow, not easier.
>>
>> +#define SETUP_GIT_ENV(env)              \
>> +	if (!env)                       \
>> +		setup_git_env();        \
>> +	return env;
>> +
>>  static char *expand_namespace(const char *raw_namespace)
>>  {
>>  	struct strbuf buf = STRBUF_INIT;
>> @@ -199,9 +204,7 @@ int is_bare_repository(void)
>>  
>>  const char *get_git_dir(void)
>>  {
>> -	if (!git_dir)
>> -		setup_git_env();
>> -	return git_dir;
>> +	SETUP_GIT_ENV(git_dir);
>>  }

Having said that, I do think a higher-level macro that encapulates the whole thing may not be such a bad idea, i.e. making the above into

    DECLARE_GIT_GETTER(const char *, get_git_dir, git_dir)

and this and others

Show 7 quoted lines

>>  char *get_object_directory(void)
>>  {
>> -	if (!git_object_dir)
>> -		setup_git_env();
>> -	return git_object_dir;
>> +	SETUP_GIT_ENV(git_object_dir);
>>  }

into

    DECLARE_GIT_GETTER(char *, get_object_directory, git_object_dir)
    DECLARE_GIT_GETTER(char *, get_index_file, git_index_file)
    DECLARE_GIT_GETTER(char *, get_graft_file, git_graft_file)