All of lore.kernel.org
 help / color / mirror / 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>,
	"Matthieu Moy" <Matthieu.Moy@grenoble-inp.fr>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"Ramsay Jones" <ramsay@ramsayjones.plus.com>,
	"Johannes Sixt" <j6t@kdbg.org>, "René Scharfe" <l.s.r@web.de>,
	"Christian Couder" <chriscool@tuxfamily.org>
Subject: [PATCH v7 04/40] builtin/apply: make find_header() return -1 instead of die()ing
Date: Mon, 13 Jun 2016 18:09:06 +0200	[thread overview]
Message-ID: <20160613160942.1806-5-chriscool@tuxfamily.org> (raw)
In-Reply-To: <20160613160942.1806-1-chriscool@tuxfamily.org>

To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.

To do that in a compatible manner with the rest of the error handling
in builtin/apply.c, find_header() should return -1 instead of calling
die().

Unfortunately find_header() already returns -1 when no header is found,
so let's make it return -2 instead in this case.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/apply.c       | 33 ++++++++++++++++++++++-----------
 t/t4254-am-corrupt.sh |  2 +-
 2 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 2ff8450..630c01c 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -1419,6 +1419,14 @@ static int parse_fragment_header(const char *line, int len, struct fragment *fra
 	return offset;
 }
 
+/*
+ * Find file diff header
+ *
+ * Returns:
+ *  -1 in case of error
+ *  -2 if no header was found
+ *   the size of the header in bytes (called "offset") otherwise
+ */
 static int find_header(struct apply_state *state,
 		       const char *line,
 		       unsigned long size,
@@ -1452,8 +1460,8 @@ static int find_header(struct apply_state *state,
 			struct fragment dummy;
 			if (parse_fragment_header(line, len, &dummy) < 0)
 				continue;
-			die(_("patch fragment without header at line %d: %.*s"),
-			    state->linenr, (int)len-1, line);
+			return error(_("patch fragment without header at line %d: %.*s"),
+				     state->linenr, (int)len-1, line);
 		}
 
 		if (size < len + 6)
@@ -1469,18 +1477,18 @@ static int find_header(struct apply_state *state,
 				continue;
 			if (!patch->old_name && !patch->new_name) {
 				if (!patch->def_name)
-					die(Q_("git diff header lacks filename information when removing "
-					       "%d leading pathname component (line %d)",
-					       "git diff header lacks filename information when removing "
-					       "%d leading pathname components (line %d)",
-					       state->p_value),
-					    state->p_value, state->linenr);
+					return error(Q_("git diff header lacks filename information when removing "
+							"%d leading pathname component (line %d)",
+							"git diff header lacks filename information when removing "
+							"%d leading pathname components (line %d)",
+							state->p_value),
+						     state->p_value, state->linenr);
 				patch->old_name = xstrdup(patch->def_name);
 				patch->new_name = xstrdup(patch->def_name);
 			}
 			if (!patch->is_delete && !patch->new_name)
-				die("git diff header lacks filename information "
-				    "(line %d)", state->linenr);
+				return error("git diff header lacks filename information "
+					     "(line %d)", state->linenr);
 			patch->is_toplevel_relative = 1;
 			*hdrsize = git_hdr_len;
 			return offset;
@@ -1505,7 +1513,7 @@ static int find_header(struct apply_state *state,
 		state->linenr += 2;
 		return offset;
 	}
-	return -1;
+	return -2;
 }
 
 static void record_ws_error(struct apply_state *state,
@@ -1996,6 +2004,9 @@ static int parse_chunk(struct apply_state *state, char *buffer, unsigned long si
 	int hdrsize, patchsize;
 	int offset = find_header(state, buffer, size, &hdrsize, patch);
 
+	if (offset == -1)
+		exit(1);
+
 	if (offset < 0)
 		return offset;
 
diff --git a/t/t4254-am-corrupt.sh b/t/t4254-am-corrupt.sh
index 85716dd..9bd7dd2 100755
--- a/t/t4254-am-corrupt.sh
+++ b/t/t4254-am-corrupt.sh
@@ -29,7 +29,7 @@ test_expect_success 'try to apply corrupted patch' '
 '
 
 test_expect_success 'compare diagnostic; ensure file is still here' '
-	echo "fatal: git diff header lacks filename information (line 4)" >expected &&
+	echo "error: git diff header lacks filename information (line 4)" >expected &&
 	test_path_is_file f &&
 	test_cmp expected actual
 '
-- 
2.9.0.rc2.411.g3e2ca28

  parent reply	other threads:[~2016-06-13 16:12 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
2016-06-13 16:09 ` [PATCH v7 01/40] apply: move 'struct apply_state' to apply.h Christian Couder
2016-06-13 22:49   ` Junio C Hamano
2016-06-14 11:07     ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 02/40] builtin/apply: make apply_patch() return -1 instead of die()ing Christian Couder
2016-06-13 22:55   ` Junio C Hamano
2016-06-14 11:06     ` Christian Couder
2016-06-14 17:50       ` Junio C Hamano
2016-06-13 16:09 ` [PATCH v7 03/40] builtin/apply: read_patch_file() " Christian Couder
2016-06-13 16:09 ` Christian Couder [this message]
2016-06-13 16:09 ` [PATCH v7 05/40] builtin/apply: make parse_chunk() return a negative integer on error Christian Couder
2016-06-13 16:09 ` [PATCH v7 06/40] builtin/apply: make parse_single_patch() return -1 " Christian Couder
2016-06-13 16:09 ` [PATCH v7 07/40] builtin/apply: make parse_whitespace_option() return -1 instead of die()ing Christian Couder
2016-06-13 16:09 ` [PATCH v7 08/40] builtin/apply: make parse_ignorewhitespace_option() " Christian Couder
2016-06-13 16:09 ` [PATCH v7 09/40] builtin/apply: move init_apply_state() to apply.c Christian Couder
2016-06-13 16:09 ` [PATCH v7 10/40] apply: make init_apply_state() return -1 instead of exit()ing Christian Couder
2016-06-13 16:09 ` [PATCH v7 11/40] builtin/apply: make check_apply_state() return -1 instead of die()ing Christian Couder
2016-06-13 16:09 ` [PATCH v7 12/40] builtin/apply: move check_apply_state() to apply.c Christian Couder
2016-06-13 16:09 ` [PATCH v7 13/40] builtin/apply: make apply_all_patches() return -1 on error Christian Couder
2016-06-13 16:09 ` [PATCH v7 14/40] builtin/apply: make parse_traditional_patch() " Christian Couder
2016-06-13 16:09 ` [PATCH v7 15/40] builtin/apply: make gitdiff_*() return 1 at end of header Christian Couder
2016-06-13 16:09 ` [PATCH v7 16/40] builtin/apply: make gitdiff_*() return -1 on error Christian Couder
2016-06-13 16:09 ` [PATCH v7 17/40] builtin/apply: change die_on_unsafe_path() to check_unsafe_path() Christian Couder
2016-06-13 16:09 ` [PATCH v7 18/40] builtin/apply: make build_fake_ancestor() return -1 on error Christian Couder
2016-06-13 16:09 ` [PATCH v7 19/40] builtin/apply: make remove_file() " Christian Couder
2016-06-13 16:09 ` [PATCH v7 20/40] builtin/apply: make add_conflicted_stages_file() " Christian Couder
2016-06-13 16:09 ` [PATCH v7 21/40] builtin/apply: make add_index_file() " Christian Couder
2016-06-13 16:09 ` [PATCH v7 22/40] builtin/apply: make create_file() " Christian Couder
2016-06-13 16:09 ` [PATCH v7 23/40] builtin/apply: make write_out_one_result() " Christian Couder
2016-06-13 16:09 ` [PATCH v7 24/40] builtin/apply: make write_out_results() " Christian Couder
2016-06-13 16:09 ` [PATCH v7 25/40] builtin/apply: make try_create_file() " Christian Couder
2016-06-13 16:09 ` [PATCH v7 26/40] builtin/apply: make create_one_file() " Christian Couder
2016-06-13 16:09 ` [PATCH v7 27/40] builtin/apply: rename option parsing functions Christian Couder
2016-06-13 16:09 ` [PATCH v7 28/40] apply: rename and move opt constants to apply.h Christian Couder
2016-06-13 16:09 ` [PATCH v7 30/40] apply: make some parsing functions static again Christian Couder
2016-06-13 16:09 ` [PATCH v7 31/40] environment: add set_index_file() Christian Couder
2016-06-13 16:09 ` [PATCH v7 32/40] write_or_die: use warning() instead of fprintf(stderr, ...) Christian Couder
2016-06-13 16:09 ` [PATCH v7 33/40] apply: add 'be_silent' variable to 'struct apply_state' Christian Couder
2016-06-13 16:09 ` [PATCH v7 34/40] apply: make 'be_silent' incompatible with 'apply_verbosely' Christian Couder
2016-06-13 16:09 ` [PATCH v7 35/40] apply: don't print on stdout when be_silent is set Christian Couder
2016-06-13 16:09 ` [PATCH v7 36/40] usage: add set_warn_routine() Christian Couder
2016-06-13 16:09 ` [PATCH v7 37/40] usage: add get_error_routine() and get_warn_routine() Christian Couder
2016-06-13 16:09 ` [PATCH v7 38/40] apply: change error_routine when be_silent is set Christian Couder
2016-06-13 16:09 ` [PATCH v7 39/40] builtin/am: use apply api in run_apply() Christian Couder
2016-06-13 16:09 ` [PATCH v7 40/40] apply: use error_errno() where possible Christian Couder
2016-06-13 16:16 ` [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder

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

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

  git send-email \
    --in-reply-to=20160613160942.1806-5-chriscool@tuxfamily.org \
    --to=christian.couder@gmail.com \
    --cc=Matthieu.Moy@grenoble-inp.fr \
    --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=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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.