Re: [PATCH] exec_cmd: system_path memory leak fix

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 24, 2014, 07:37 UTC
Message-ID: <CAPc5daWnJCGPTMA0Hyg8QEAmsnF17ZQ9njGq8xVaUAfuwCRQDA@mail.gmail.com>
In-Reply-To: <87mw7haxdp.fsf@gmail.com>

[jc: added those who were mentioned but were missing back to Cc]

On Sun, Nov 23, 2014 at 11:02 PM, Alex Kuleshov <kuleshovmail@gmail.com> wrote:

Show 8 quoted lines

>
> Junio C Hamano:
>
>>Fixing these callers are done as separate patches, that can be
>>applied either before or after this patch.
>
> How to do it better? Update this patch, fix all callers which broken and
> concat this patches to one or make separate patches?

As I said, I do not think the approach your v2 takes is better than the original approach to pass the ownership of the returned value to the caller. I'd say that a cleaned up v1 that makes sure it adds a necessary strdup() in the codepath where it returns an absolute pathname given as-is, with necessary changes to callers that do not currently free the received result to free it when they are done, and to callers that currently do strdup() of the received result not to do strdup(), in a single patch, would be the right thing to do.

I think I already wrote the bulk of proposed commit message for you for such a change earlier ;-) The one that talks about changing the contract between the system_path() and its callers.

Thanks.