Re: [PATCH] builtin/commit.c: memoize git-path for COMMIT_EDITMSG

[Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>]

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

> Pranit Bauva <pranit.bauva@gmail.com> writes:
>
>>  static const char *use_message_buffer;
>> -static const char commit_editmsg[] = "COMMIT_EDITMSG";
>> +static const char commit_editmsg_path[] = git_path_commit_editmsg();
>
> The function defined with the macro looks like
>
> 	const char *git_path_commit_editmsg(void)
>         {
> 		static char *ret;
>                 if (!ret)
>                 	ret = git_pathdup("COMMIT_EDITMSG");
> 		return ret;
> 	}
>
> so receiving its result to "const char v[]" looks somewhat
> suspicious.
>
> More importantly, when is this function evaluated and returned value
> used to fill commit_editmsg_path[]?

I may have missed something, but I'd say "never", as the code is not
compilable at least with my gcc:

builtin/commit.c:98:1: error: invalid initializer
 static const char commit_editmsg_path[] = git_path_commit_editmsg();
 ^

AFAIK, initializing a global variable with a function call is allowed in
C++, but not in C.

And indeed, this construct is a huge source of trouble, as it would mean
that git_path_commit_editmsg() is called 1) unconditionnally, and 2)
before entering main().

1) means that the function call is made even when git is called for
another command. This is terrible for the startup time: if all git
commands have a not-totally-immediate initializer, then all commands
would need to run the initializers for all other commands. 2) means it's
a nightmare to debug, as you can hardly predict when the code will be
executed.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/