builtin/apply: make apply_all_patches() return 128 or 1 on error

To finish libifying the apply functionality, apply_all_patches() should not
die() or exit() in case of error, but return either 128 or 1, so that it
gives the same exit code as when die() or exit(1) is called. This way
scripts relying on the exit code don't need to be changed.

While doing that we must take care that file descriptors are properly closed
and, if needed, reset to a sensible value.

Also, according to the lockfile API, when finished with a lockfile, one
should either commit it or roll it back.

This is even more important now that the same lockfile can be passed
to init_apply_state() many times to be reused by series of calls to
the apply lib functions.

Helped-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/apply.c b/builtin/apply.c
index 075ada47d457794442394c39a3deb4ee2678fcb4..5530ba13ecc72247fadea0f34fd2b7756f72d497 100644 (file)
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4578,15 +4578,18 @@ static int apply_all_patches(struct apply_state *state,
                                              arg);
 
                fd = open(arg, O_RDONLY);
-               if (fd < 0)
-                       die_errno(_("can't open patch '%s'"), arg);
+               if (fd < 0) {
+                       error(_("can't open patch '%s': %s"), arg, strerror(errno));
+                       res = -128;
+                       goto end;
+               }
                read_stdin = 0;
                set_default_whitespace_mode(state);
                res = apply_patch(state, fd, arg, options);
+               close(fd);
                if (res < 0)
                        goto end;
                errs |= res;
-               close(fd);
        }
        set_default_whitespace_mode(state);
        if (read_stdin) {
@@ -4606,11 +4609,14 @@ static int apply_all_patches(struct apply_state *state,
                                   squelched),
                                squelched);
                }
-               if (state->ws_error_action == die_on_ws_error)
-                       die(Q_("%d line adds whitespace errors.",
-                              "%d lines add whitespace errors.",
-                              state->whitespace_error),
-                           state->whitespace_error);
+               if (state->ws_error_action == die_on_ws_error) {
+                       error(Q_("%d line adds whitespace errors.",
+                                "%d lines add whitespace errors.",
+                                state->whitespace_error),
+                             state->whitespace_error);
+                       res = -128;
+                       goto end;
+               }
                if (state->applied_after_fixing_ws && state->apply)
                        warning("%d line%s applied after"
                                " fixing whitespace errors.",
@@ -4624,15 +4630,24 @@ 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);
+               if (res) {
+                       error(_("Unable to write new index file"));
+                       res = -128;
+                       goto end;
+               }
                state->newfd = -1;
        }
 
        return !!errs;
 
 end:
-       exit(res == -1 ? 1 : 128);
+       if (state->newfd >= 0) {
+               rollback_lock_file(state->lock_file);
+               state->newfd = -1;
+       }
+
+       return (res == -1 ? 1 : 128);
 }
 
 int cmd_apply(int argc, const char **argv, const char *prefix)