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

[Junio C Hamano <gitster@pobox.com>]

Eric Sunshine <sunshine@sunshineco.com> writes:

>> +       /*
>> +        * Since lockfile.c keeps a linked list of all created
>> +        * lock_file structures, it isn't safe to free(lock_file).
>> +        */
>> +       struct lock_file *lock_file;
>
> Is there ever a time when lock_file is removed from the list (such as
> upon successful write of the index), in which case it would be safe to
> free() this?

I do not think you need to think about "free"ing.

Even if the libified version of the apply internal can be called
multiple times to process multiple patch inputs, there is no need to
run multiple instances of it yet.  And a lockfile, after the caller
finishes interacting with one file using it by calling commit or
rollback, can be reused to interact with another file.

So the cleanest would be to:

 * make the caller of apply API responsible for preparing a static
   or (allocating and leaking) lockfile instance,

 * make it pass a pointer to the lockfile to the apply API so that
   it can store it in apply_state, and

 * have the caller use apply API feeding one patch at a time in a
   loop, allowing the same lockfile instance used for each
   "invocation".

I sounded as if I were advocating non-reentrant implementation in
the introductory paragraph, but that is not the intention.  If you
want to do N threads, you would prepare N lockfile instances, give
them to N apply API instances to be stored in N apply_state
instances, and you would have N parallel applications, if you wanted
to.