Re: [PATCH v5 12/44] builtin-am: implement --skip

From: Stefan Beller <sbeller@google.com>
Date: Jul 14, 2015, 16:54 UTC
Message-ID: <CAGZ79kYYJughaqBD7hzzDBO4xmrJ7P3VT7m=qG-SRLXiK-f_vA@mail.gmail.com>
In-Reply-To: <CACRoPnRri5Nu5rj9O7id+PyiDBJpZkBDk8kB0WCuB3Uef76M3g@mail.gmail.com>

On Tue, Jul 14, 2015 at 2:34 AM, Paul Tan <pyokagan@gmail.com> wrote:

Show 14 quoted lines

> On Tue, Jul 14, 2015 at 3:05 AM, Stefan Beller <sbeller@google.com> wrote:
>> All returns before this point leak the memory of `lock_file`.
>
> Yeah, it's intentional. From Documentation/technical/api-lockfile.txt:
>
> * Allocates a `struct lock_file` either as a static variable or on the
>   heap, initialized to zeros. Once you use the structure to call the
>   `hold_lock_file_*` family of functions, it belongs to the lockfile
>   subsystem and its storage must remain valid throughout the life of
>   the program (i.e. you cannot use an on-stack variable to hold this
>   structure).
>
> Thanks,
> Paul

So what I meant to suggest, was to only allocate the memory if we really need it by moving the allocation further down.

static int clean_index(const unsigned char *head, const unsigned char *remote)
{
       struct lock_file *lock_file;
...
... // includes return -1, which would not leak the memory already allocated
...
       lock_file = xalloc (...);
       hold_locked_index(lock_file, 1);