Re: [PATCH v3 48/49] builtin/apply: move 'lock_file' global into 'struct apply_state'

> Christian Couder <christian.couder@gmail.com> writes:
>
>> We cannot have a 'struct lock_file' allocated on the stack, as lockfile.c
>> keeps a linked list of all created lock_file structures. So let's make the
>> 'lock_file' variable a pointer to a 'struct lock_file'
>
> Good.
>
>> As the same instance of this struct can be reused, let's add an argument
>> to init_apply_state(), so that the caller can supply the same instance to
>> different calls.
>
> Good.
>
>> And let's alloc an instance in init_apply_state(), if the
>> caller doesn't want to supply one.
>
> This is questionable.
>
>> -static struct lock_file lock_file;
>
> I'd rather leave this as-is, and pass the pointer to it to
> init_apply_state().  For the purpose of "cmd_apply()" which is the
> first user of the "(semi-)libified apply API", that reads a single
> patch and applies it before exiting, that is sufficient.

> As to the "if the caller does not want to supply one, we will
> allocate one that will definitely be leaked", I am mildly opposed.
>
> By making it the responsibility of the caller, whenever a new caller
> of the libified apply API is written, those who write it is _forced_
> to think about not leaking the lockfile structure, which is a good
> thing.