On Wed, May 11, 2016 at 9:17 AM, Christian Couder
<christian.couder@xxxxxxxxx> 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@xxxxxxxxxxxxx>
> ---
> 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.)
> + if (res)
> + return error(_("Unable to write new index file"));
> }
>
> return !!errs;
--
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