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

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 23, 2014, 22:43 UTC
Message-ID: <xmqq61jrb62x.fsf@gitster.dls.corp.google.com>
In-Reply-To: <CAPig+cTyZxH6Ka3JVpYBP7=i=HAWoSYBt9MScwfVaTcKKmi1UA@mail.gmail.com>

Eric Sunshine <sunshine@sunshineco.com> writes:

Show 13 quoted lines

> On Thu, Jun 19, 2014 at 5:28 PM, Jeff King <peff@peff.net> wrote:
>> This is shorter, harder to get wrong, and more clearly
>> captures the intent.
>>
>> Signed-off-by: Jeff King <peff@peff.net>
>> ---
>> 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 ;-).

Show 32 quoted lines

>
> [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