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 <pclo...@gmail.com>
Helped-by: Johannes Schindelin <johannes.schinde...@gmx.de>
Helped-by: Eric Sunshine <sunsh...@sunshineco.com>
Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
---
 builtin/apply.c | 37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 18c5419..29ca524 100644
--- 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)
-- 
2.9.0.172.gfb57a78

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to