git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v10 14/40] builtin/apply: make apply_all_patches() return 128 or 1 on error

From
Christian Couder <christian.couder@gmail.com>
Date
Aug 8, 2016, 21:03 UTC
Message-ID
<20160808210337.5038-15-chriscool@tuxfamily.org>
In-Reply-To
<20160808210337.5038-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.9.2.614.g4980f51
Previous: Christian CouderNext: Christian Couder
Message 44 of 51 in “libify apply and use lib in am, part 2”
  1. 00/40 libify apply and use lib in am, part 2Christian Couder, Aug 8, 2016
  2. 02/40 apply: move 'struct apply_state' to apply.hChristian Couder, Aug 8, 2016
  3. 03/40 builtin/apply: make apply_patch() return -1 or -128 instead of die()ingChristian Couder, Aug 8, 2016
  4. 04/40 builtin/apply: read_patch_file() return -1 instead of die()ingChristian Couder, Aug 8, 2016
  5. 07/40 builtin/apply: make parse_single_patch() return -1 on errorChristian Couder, Aug 8, 2016
  6. 05/40 builtin/apply: make find_header() return -128 instead of die()ingChristian Couder, Aug 8, 2016
  7. 06/40 builtin/apply: make parse_chunk() return a negative integer on errorChristian Couder, Aug 8, 2016
  8. 10/40 builtin/apply: move init_apply_state() to apply.cChristian Couder, Aug 8, 2016
  9. 09/40 builtin/apply: make parse_ignorewhitespace_option() return -1 instead of die()ingChristian Couder, Aug 8, 2016
  10. 12/40 builtin/apply: make check_apply_state() return -1 instead of die()ingChristian Couder, Aug 8, 2016
  11. 13/40 builtin/apply: move check_apply_state() to apply.cChristian Couder, Aug 8, 2016
  12. 11/40 apply: make init_apply_state() return -1 instead of exit()ingChristian Couder, Aug 8, 2016
  13. 15/40 builtin/apply: make parse_traditional_patch() return -1 on errorChristian Couder, Aug 8, 2016
  14. 21/40 builtin/apply: make add_conflicted_stages_file() return -1 on errorChristian Couder, Aug 8, 2016
  15. 20/40 builtin/apply: make remove_file() return -1 on errorChristian Couder, Aug 8, 2016
  16. 22/40 builtin/apply: make add_index_file() return -1 on errorChristian Couder, Aug 8, 2016
  17. 23/40 builtin/apply: make create_file() return -1 on errorChristian Couder, Aug 8, 2016
  18. 27/40 builtin/apply: make create_one_file() return -1 on errorChristian Couder, Aug 8, 2016
  19. 29/40 apply: rename and move opt constants to apply.hChristian Couder, Aug 8, 2016
  20. 28/40 builtin/apply: rename option parsing functionsChristian Couder, Aug 8, 2016
  21. stefan.naewe@atlas-elektronik.comAug 9, 2016
  22. 26/40 builtin/apply: make try_create_file() return -1 on errorChristian Couder, Aug 8, 2016
  23. 25/40 builtin/apply: make write_out_results() return -1 on errorChristian Couder, Aug 8, 2016
  24. 31/40 apply: make some parsing functions static againChristian Couder, Aug 8, 2016
  25. 24/40 builtin/apply: make write_out_one_result() return -1 on errorChristian Couder, Aug 8, 2016
  26. 32/40 apply: use error_errno() where possibleChristian Couder, Aug 8, 2016
  27. 19/40 builtin/apply: make build_fake_ancestor() return -1 on errorChristian Couder, Aug 8, 2016
  28. 37/40 usage: add get_error_routine() and get_warn_routine()Christian Couder, Aug 8, 2016
  29. 36/40 usage: add set_warn_routine()Christian Couder, Aug 8, 2016
  30. 35/40 apply: don't print on stdout in verbosity_silent modeChristian Couder, Aug 8, 2016
  31. 39/40 apply: refactor `git apply` option parsingChristian Couder, Aug 8, 2016
  32. 33/40 environment: add set_index_file()Christian Couder, Aug 8, 2016
  33. Junio C HamanoAug 8, 2016
  34. Christian CouderAug 10, 2016
  35. Junio C HamanoAug 10, 2016
  36. Christian CouderAug 11, 2016
  37. Junio C HamanoAug 11, 2016
  38. 34/40 apply: make it possible to silently applyChristian Couder, Aug 8, 2016
  39. 38/40 apply: change error_routine when silentChristian Couder, Aug 8, 2016
  40. 18/40 builtin/apply: change die_on_unsafe_path() to check_unsafe_path()Christian Couder, Aug 8, 2016
  41. 40/40 builtin/am: use apply api in run_apply()Christian Couder, Aug 8, 2016
  42. 17/40 builtin/apply: make gitdiff_*() return -1 on errorChristian Couder, Aug 8, 2016
  43. 16/40 builtin/apply: make gitdiff_*() return 1 at end of headerChristian Couder, Aug 8, 2016
  44. 14/40 builtin/apply: make apply_all_patches() return 128 or 1 on errorChristian Couder, Aug 8, 2016
  45. 08/40 builtin/apply: make parse_whitespace_option() return -1 instead of die()ingChristian Couder, Aug 8, 2016
  46. 01/40 apply: make some names more specificChristian Couder, Aug 8, 2016
  47. stefan.naewe@atlas-elektronik.comAug 9, 2016
  48. Christian CouderAug 11, 2016
  49. stefan.naewe@atlas-elektronik.comAug 11, 2016
  50. Christian CouderAug 8, 2016
  51. Junio C HamanoAug 8, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.