Re: [PATCH v2 63/94] builtin/apply: make apply_all_patches() return -1 on error
From: Eric Sunshine <hidden>
Date: 2016-06-16 02:19:24
On Wed, May 11, 2016 at 9:17 AM, Christian Couder [off-list ref] wrote:
quoted hunk ↗ jump to hunk
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 <redacted> ---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;