Re: [PATCH v2 63/94] builtin/apply: make apply_all_patches() return -1 on error

> On Wed, May 11, 2016 at 9:17 AM, Christian Couder
> <christian.couder@gmail.com> wrote:
>> To finish libifying the apply functionality, apply_all_patches() should not
>> die() or exit() in case of error, but return -1.
>>
>> While doing that we must take care that file descriptors are properly closed
>> and, if needed, reset a sensible value.
>>
>> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
>> ---
>> diff --git a/builtin/apply.c b/builtin/apply.c
>> @@ -4613,9 +4613,10 @@ static int apply_all_patches(struct apply_state *state,
>>         }
>>
>>         if (state->update_index) {
>> -               if (write_locked_index(&the_index, state->lock_file, COMMIT_LOCK))
>> -                       die(_("Unable to write new index file"));
>> +               res = write_locked_index(&the_index, state->lock_file, COMMIT_LOCK);
>>                 state->newfd = -1;
>
> Does write_locked_index() unconditionally close the file descriptor
> even when an error occurs? If not, then isn't this potentially leaking
> 'newfd'?
>
> (My very cursory read of write_locked_index() seems to reveal that the
> file descriptor may indeed remain open upon index write failure.)