On Fri, Jun 3, 2016 at 8:03 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Christian Couder <christian.couder@xxxxxxxxx> writes:
>
>> This is to replace:
>>
>> "[PATCH v3 48/49] builtin/apply: move 'lock_file' global into 'struct apply_state'"
>>
>> from the "libify apply and use lib in am, part 1" patch series.
>
> Thanks; will replace the tip 2 patches and requeue.
>
>> diff --git a/builtin/apply.c b/builtin/apply.c
>> index 5027f1b..cc635eb 100644
>> --- a/builtin/apply.c
>> +++ b/builtin/apply.c
>> @@ -52,6 +52,12 @@ struct apply_state {
>> const char *prefix;
>> int prefix_length;
>>
>> + /*
>> + * 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 this even an issue for this thing anymore? It is the
> responsibilty of the API user to manage the lifetime of what
> lock_file points at. The caller may have it on heap and let it
> leak, or it may have in BSS in which case it won't even dream about
> freeing it.
>
> If a comment were needed for this field, it should say "when
> discarding/freeing apply_state, just discard this pointer without
> touching what the pointer points at; the storage the pointer points
> at does not belong to the API implementation but belongs to the API
> user".
>
> But I do not think such a comment is needed, because the situation
> is the same as other fields like *prefix, and for the same reason we
> do not do anything to these fields in clear_apply_state().
Ok, I just resent without this comment.
I still don't understand why there is an added:
From: Christian Couder <christian.couder@xxxxxxxxx>
at the beginning of the emails.
> Other than that this looks great.
Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html