Re: [PATCH v2 23/25] name-rev: avoid leaking memory in the `deref` case
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 4, 2017, 04:22 UTC
- Message-ID
- <xmqq60hhb69x.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <alpine.DEB.2.20.1705021433060.3480@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 34 quoted lines
> Hi Junio,
>
> On Mon, 1 May 2017, Junio C Hamano wrote:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>> > Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>> >
>> >> diff --git a/builtin/name-rev.c b/builtin/name-rev.c
>> >> index 92a5d8a5d26..a4ce73fb1e9 100644
>> >> --- a/builtin/name-rev.c
>> >> +++ b/builtin/name-rev.c
>> >> @@ -28,6 +28,7 @@ static void name_rev(struct commit *commit,
>> >> struct rev_name *name = (struct rev_name *)commit->util;
>> >> struct commit_list *parents;
>> >> int parent_number = 1;
>> >> + char *p = NULL;
>> >>
>> >> parse_commit(commit);
>> >>
>> >> @@ -35,7 +36,7 @@ static void name_rev(struct commit *commit,
>> >> return;
>> >>
>> >> if (deref) {
>> >> - tip_name = xstrfmt("%s^0", tip_name);
>> >> + tip_name = p = xstrfmt("%s^0", tip_name);
>>
>> I'll rename 'p' to 'to_free' while queuing, though. Without a
>> descriptive name, it was confusing to view while resolving conflicts
>> with another in-flight topic.
>
> Good point. I also used `p` in builtin/mktree.c and setup.c. While you
> seem to have renamed it in builtin/mktree.c, I think setup.c also needs
> the same fixup.Yes, no question about it. I just left it as-is as I saw it is likely to be rerolled anyway due to other issues ;-)