git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Christian Couder <christian.couder@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>, "Jeff King" <peff@peff.net>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Karsten Blees" <karsten.blees@gmail.com>,
	"Nguyen Thai Ngoc Duy" <pclouds@gmail.com>,
	"Stefan Beller" <sbeller@google.com>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"Ramsay Jones" <ramsay@ramsayjones.plus.com>,
	"Johannes Sixt" <j6t@kdbg.org>, "René Scharfe" <l.s.r@web.de>,
	"Stefan Naewe" <stefan.naewe@atlas-elektronik.com>,
	"Christian Couder" <chriscool@tuxfamily.org>
Subject: [PATCH v14 14/41] builtin/apply: make apply_all_patches() return 128 or 1 on error
Date: Sun,  4 Sep 2016 22:18:06 +0200	[thread overview]
Message-ID: <20160904201833.21676-15-chriscool@tuxfamily.org> (raw)
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>

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>
---
 builtin/apply.c | 37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 075ada4..5530ba1 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.10.0.41.g9df52c3


  parent reply	other threads:[~2016-09-04 20:19 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-04 20:17 [PATCH v14 00/41] libify apply and use lib in am, part 2 Christian Couder
2016-09-04 20:17 ` [PATCH v14 01/41] apply: make some names more specific Christian Couder
2016-09-04 20:17 ` [PATCH v14 02/41] apply: move 'struct apply_state' to apply.h Christian Couder
2016-09-04 20:17 ` [PATCH v14 03/41] builtin/apply: make apply_patch() return -1 or -128 instead of die()ing Christian Couder
2016-09-04 20:17 ` [PATCH v14 04/41] builtin/apply: read_patch_file() return -1 " Christian Couder
2016-09-04 20:17 ` [PATCH v14 05/41] builtin/apply: make find_header() return -128 " Christian Couder
2016-09-04 20:17 ` [PATCH v14 06/41] builtin/apply: make parse_chunk() return a negative integer on error Christian Couder
2016-09-04 20:17 ` [PATCH v14 07/41] builtin/apply: make parse_single_patch() return -1 " Christian Couder
2016-09-04 20:18 ` [PATCH v14 08/41] builtin/apply: make parse_whitespace_option() return -1 instead of die()ing Christian Couder
2016-09-04 20:18 ` [PATCH v14 09/41] builtin/apply: make parse_ignorewhitespace_option() " Christian Couder
2016-09-04 20:18 ` [PATCH v14 10/41] builtin/apply: move init_apply_state() to apply.c Christian Couder
2016-09-04 20:18 ` [PATCH v14 11/41] apply: make init_apply_state() return -1 instead of exit()ing Christian Couder
2016-09-04 20:18 ` [PATCH v14 12/41] builtin/apply: make check_apply_state() return -1 instead of die()ing Christian Couder
2016-09-04 20:18 ` [PATCH v14 13/41] builtin/apply: move check_apply_state() to apply.c Christian Couder
2016-09-04 20:18 ` Christian Couder [this message]
2016-09-04 20:18 ` [PATCH v14 15/41] builtin/apply: make parse_traditional_patch() return -1 on error Christian Couder
2016-09-04 20:18 ` [PATCH v14 16/41] builtin/apply: make gitdiff_*() return 1 at end of header Christian Couder
2016-09-04 20:18 ` [PATCH v14 17/41] builtin/apply: make gitdiff_*() return -1 on error Christian Couder
2016-09-04 20:18 ` [PATCH v14 18/41] builtin/apply: change die_on_unsafe_path() to check_unsafe_path() Christian Couder
2016-09-04 20:18 ` [PATCH v14 19/41] builtin/apply: make build_fake_ancestor() return -1 on error Christian Couder
2016-09-04 20:18 ` [PATCH v14 20/41] builtin/apply: make remove_file() " Christian Couder
2016-09-04 20:18 ` [PATCH v14 21/41] builtin/apply: make add_conflicted_stages_file() " Christian Couder
2016-09-04 20:18 ` [PATCH v14 22/41] builtin/apply: make add_index_file() " Christian Couder
2016-09-04 20:18 ` [PATCH v14 23/41] builtin/apply: make create_file() " Christian Couder
2016-09-04 20:18 ` [PATCH v14 24/41] builtin/apply: make write_out_one_result() " Christian Couder
2016-09-04 20:18 ` [PATCH v14 25/41] builtin/apply: make write_out_results() " Christian Couder
2016-09-04 20:18 ` [PATCH v14 26/41] builtin/apply: make try_create_file() " Christian Couder
2016-09-04 20:18 ` [PATCH v14 27/41] builtin/apply: make create_one_file() " Christian Couder
2016-09-04 20:18 ` [PATCH v14 28/41] builtin/apply: rename option parsing functions Christian Couder
2016-09-04 20:18 ` [PATCH v14 29/41] apply: rename and move opt constants to apply.h Christian Couder
2016-09-04 20:18 ` [PATCH v14 31/41] apply: make some parsing functions static again Christian Couder
2016-09-04 20:18 ` [PATCH v14 32/41] apply: use error_errno() where possible Christian Couder
2016-09-04 20:18 ` [PATCH v14 33/41] apply: make it possible to silently apply Christian Couder
2016-09-04 20:18 ` [PATCH v14 34/41] apply: don't print on stdout in verbosity_silent mode Christian Couder
2016-09-04 20:18 ` [PATCH v14 35/41] usage: add set_warn_routine() Christian Couder
2016-09-04 20:18 ` [PATCH v14 36/41] usage: add get_error_routine() and get_warn_routine() Christian Couder
2016-09-04 20:18 ` [PATCH v14 37/41] apply: change error_routine when silent Christian Couder
2016-09-04 20:18 ` [PATCH v14 38/41] apply: refactor `git apply` option parsing Christian Couder
2016-09-04 20:18 ` [PATCH v14 39/41] apply: pass apply state to build_fake_ancestor() Christian Couder
2016-09-04 20:18 ` [PATCH v14 40/41] apply: learn to use a different index file Christian Couder
2016-09-04 20:18 ` [PATCH v14 41/41] builtin/am: use apply API in run_apply() Christian Couder
     [not found] ` <20160904201833.21676-31-chriscool@tuxfamily.org>
2016-09-06 18:29   ` [PATCH v14 30/41] Move libified code from builtin/apply.c to apply.{c,h} Stefan Beller
2016-09-06 18:54 ` [PATCH v14 00/41] libify apply and use lib in am, part 2 Stefan Beller
2016-09-07 19:51 ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160904201833.21676-15-chriscool@tuxfamily.org \
    --to=christian.couder@gmail.com \
    --cc=avarab@gmail.com \
    --cc=chriscool@tuxfamily.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    --cc=karsten.blees@gmail.com \
    --cc=l.s.r@web.de \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    --cc=ramsay@ramsayjones.plus.com \
    --cc=sbeller@google.com \
    --cc=stefan.naewe@atlas-elektronik.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).