Subject: Re: [PATCH v2 06/10] setup_git_env: use git_pathdup instead of xmalloc + sprintf

From: Junio C Hamano <[email-masked]>
Date: Mon, 23 Jun 2014 15:43:02 -0700

Eric Sunshine <[email-masked]> writes:

> On Thu, Jun 19, 2014 at 5:28 PM, Jeff King <[email-masked]> wrote:
>> This is shorter, harder to get wrong, and more clearly
>> captures the intent.
>>
>> Signed-off-by: Jeff King <[email-masked]>
>> ---
>> I wondered if there was a reason to avoid this (because we are in
>> setup_git_env, which can potentially be called by git_pathdup). But the
>> git_graft_file initialization below already uses it, and I
>> double-checked that it is safe once git_dir is set.
>
> This patch will conflict textually with patch 6/28 of Duy's
> nd/multiple-work-trees series [1].

Thanks; I noticed that and dropped the other topic tentatively, as
it is being rerolled anyway.  In addition to that, because this
series seems fairly focused and well done, and the owners of two
topics known to be competent and active folks, I do not think there
is not much to be worried about ;-).

>
> [1]: http://thread.gmane.org/gmane.comp.version-control.git/242300/focus=243649
>
>>  environment.c | 12 ++++--------
>>  1 file changed, 4 insertions(+), 8 deletions(-)
>>
>> diff --git a/environment.c b/environment.c
>> index 4dac5e9..4de7b81 100644
>> --- a/environment.c
>> +++ b/environment.c
>> @@ -135,15 +135,11 @@ static void setup_git_env(void)
>>         gitfile = read_gitfile(git_dir);
>>         git_dir = xstrdup(gitfile ? gitfile : git_dir);
>>         git_object_dir = getenv(DB_ENVIRONMENT);
>> -       if (!git_object_dir) {
>> -               git_object_dir = xmalloc(strlen(git_dir) + 9);
>> -               sprintf(git_object_dir, "%s/objects", git_dir);
>> -       }
>> +       if (!git_object_dir)
>> +               git_object_dir = git_pathdup("objects");
>>         git_index_file = getenv(INDEX_ENVIRONMENT);
>> -       if (!git_index_file) {
>> -               git_index_file = xmalloc(strlen(git_dir) + 7);
>> -               sprintf(git_index_file, "%s/index", git_dir);
>> -       }
>> +       if (!git_index_file)
>> +               git_index_file = git_pathdup("index");
>>         git_graft_file = getenv(GRAFT_ENVIRONMENT);
>>         if (!git_graft_file)
>>                 git_graft_file = git_pathdup("info/grafts");
>> --
>> 2.0.0.566.gfe3e6b2