From: Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Date: Mon, 14 Nov 2011 03:32:11 GMT
Subject: Re: [PATCH 2/2] Copy resolve_ref() return value for longer use
Message-ID: <CACsJy8BnqoPVJiM6mbq7p3gKtLh-KGUuTshcukGokC3istTxMQ@mail.gmail.com>
In-Reply-To: <7vbosfoiuy.fsf@alter.siamese.dyndns.org>

2011/11/14 Junio C Hamano <gitster@pobox.com>:
>> diff --git a/builtin/branch.c b/builtin/branch.c
>> index 0fe9c4d..5b6d839 100644
>> --- a/builtin/branch.c
>> +++ b/builtin/branch.c
>> @@ -115,8 +115,10 @@ static int branch_merged(int kind, const char *name,
>>                   branch->merge[0] &&
>>                   branch->merge[0]->dst &&
>>                   (reference_name =
>> -                  resolve_ref(branch->merge[0]->dst, sha1, 1, NULL)) != NULL)
>> +                  resolve_ref(branch->merge[0]->dst, sha1, 1, NULL)) != NULL) {
>> +                     reference_name = xstrdup(reference_name);
>>                       reference_rev = lookup_commit_reference(sha1);
>> +             }
>>       }
>>       if (!reference_rev)
>>               reference_rev = head_rev;
>> @@ -141,6 +143,7 @@ static int branch_merged(int kind, const char *name,
>>                               "         '%s', even though it is merged to HEAD."),
>>                               name, reference_name);
>>       }
>> +     free((char*)reference_name);
>>       return merged;
>>  }
>
> Now reference_name stores the result of xstrdup(), it does not have reason
> to be of type "const char *". It is preferable to lose the cast here, I
> think. The same comment applies to the remainder of the patch.

But resolve_ref() returns "const char *", we need to type cast at
least once, either at resolve_ref() assignment or at free(), until we
change resolve_ref(). Or should we change resolve_ref() to return
"char *" now?
-- 
Duy