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

Re: [PATCH v2 1/3] am: add --show-current-patch

From
Duy Nguyen <pclouds@gmail.com>
Date
Feb 2, 2018, 09:25 UTC
Message-ID
<20180202092511.GA28946@ash>
In-Reply-To
<xmqqpo5pr5wr.fsf@gitster-ct.c.googlers.com>
On Wed, Jan 31, 2018 at 02:59:32PM -0800, Junio C Hamano wrote:
Show 27 quoted lines
> Eric Sunshine <sunshine@sunshineco.com> writes:
> 
> > On Wed, Jan 31, 2018 at 4:30 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> >> Pointing the user to $GIT_DIR/rebase-apply may encourage them to mess
> >> around in there, which is not a good thing. With this, the user does
> >> not have to keep the path around somewhere (because after a couple of
> >> commands, the path may be out of scrollback buffer) when they need to
> >> look at the patch.
> >>
> >> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> >> ---
> >> diff --git a/builtin/am.c b/builtin/am.c
> >> @@ -2121,6 +2120,22 @@ static void am_abort(struct am_state *state)
> >> +static int show_patch(struct am_state *state)
> >> +{
> >> +       struct strbuf sb = STRBUF_INIT;
> >> +       int len;
> >> +
> >> +       len = strbuf_read_file(&sb, am_path(state, msgnum(state)), 0);
> >> +       if (len < 0)
> >> +               die_errno(_("failed to read '%s'"),
> >> +                         am_path(state, msgnum(state)));
> >
> > Isn't this am_path() invocation inside die_errno() likely to clobber
> > the 'errno' from strbuf_read_file() which you want to be reporting?
> 
> True.

Thanks both. Good catch. Of course I will fix this in the re-roll, but should we also do something for the current code base with the following patch?

I think this is something coccinelle can help catch, but my coccinelle-foo is way too low to come up with something like "die_errno() must not take any arguments as function calls, except _() and N_()".

-- 8< --
Subject: [PATCH] Preserve errno in case case before calling sth_errno()
All these locations do something like this
    sth_errno(..., somefunc(...));

where somefunc() can potentially change errno, which will be read by sth_errno(). Call somefunc separately with errno preserved to avoid this.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/am.c      |  8 +++++++-
 builtin/commit.c  |  8 ++++++--
 builtin/init-db.c |  9 ++++++---
 rerere.c          |  9 ++++++---
 shallow.c         | 27 ++++++++++++++++++---------
 5 files changed, 43 insertions(+), 18 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index acfe9d3c8c..ba67e187a4 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -244,6 +244,9 @@ static int am_in_progress(const struct am_state *state)
 static int read_state_file(struct strbuf *sb, const struct am_state *state,
 			const char *file, int trim)
 {
+	int saved_errno;
+	const char *path;
+
 	strbuf_reset(sb);
 
 	if (strbuf_read_file(sb, am_path(state, file), 0) >= 0) {
@@ -256,7 +259,10 @@ static int read_state_file(struct strbuf *sb, const struct am_state *state,
 	if (errno == ENOENT)
 		return -1;
 
-	die_errno(_("could not read '%s'"), am_path(state, file));
+	saved_errno = errno;
+	path = am_path(state, file);
+	errno = saved_errno;
+	die_errno(_("could not read '%s'"), path);
 }
 
 /**
diff --git a/builtin/commit.c b/builtin/commit.c
index 4610e3d8e3..39836b3734 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -780,8 +780,12 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	}
 
 	s->fp = fopen_for_writing(git_path_commit_editmsg());
-	if (s->fp == NULL)
-		die_errno(_("could not open '%s'"), git_path_commit_editmsg());
+	if (s->fp == NULL) {
+		int saved_errno = errno;
+		const char *path = git_path_commit_editmsg();
+		errno = saved_errno;
+		die_errno(_("could not open '%s'"), path);
+	}
 
 	/* Ignore status.displayCommentPrefix: we do need comments in COMMIT_EDITMSG. */
 	old_display_comment_prefix = s->display_comment_prefix;
diff --git a/builtin/init-db.c b/builtin/init-db.c
index c9b7946bad..e384749f73 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -570,9 +570,12 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
 			set_git_work_tree(work_tree);
 		else
 			set_git_work_tree(git_work_tree_cfg);
-		if (access(get_git_work_tree(), X_OK))
-			die_errno (_("Cannot access work tree '%s'"),
-				   get_git_work_tree());
+		if (access(get_git_work_tree(), X_OK)) {
+			int saved_errno = errno;
+			const char *path = get_git_work_tree();
+			errno = saved_errno;
+			die_errno(_("Cannot access work tree '%s'"), path);
+		}
 	}
 	else {
 		if (work_tree)
diff --git a/rerere.c b/rerere.c
index 1ce440f4bb..f19a53ff2c 100644
--- a/rerere.c
+++ b/rerere.c
@@ -683,9 +683,12 @@ static int merge(const struct rerere_id *id, const char *path)
 	 * A successful replay of recorded resolution.
 	 * Mark that "postimage" was used to help gc.
 	 */
-	if (utime(rerere_path(id, "postimage"), NULL) < 0)
-		warning_errno("failed utime() on %s",
-			      rerere_path(id, "postimage"));
+	if (utime(rerere_path(id, "postimage"), NULL) < 0) {
+		int saved_errno = errno;
+		const char *path = rerere_path(id, "postimage");
+		errno = saved_errno;
+		warning_errno("failed utime() on %s", path);
+	}
 
 	/* Update "path" with the resolution */
 	f = fopen(path, "w");
diff --git a/shallow.c b/shallow.c
index df4d44ea7a..9da82f5292 100644
--- a/shallow.c
+++ b/shallow.c
@@ -295,9 +295,12 @@ const char *setup_temporary_shallow(const struct oid_array *extra)
 		temp = xmks_tempfile(git_path("shallow_XXXXXX"));
 
 		if (write_in_full(temp->fd, sb.buf, sb.len) < 0 ||
-		    close_tempfile_gently(temp) < 0)
-			die_errno("failed to write to %s",
-				  get_tempfile_path(temp));
+		    close_tempfile_gently(temp) < 0) {
+			int saved_errno = errno;
+			const char *path = get_tempfile_path(temp);
+			errno = saved_errno;
+			die_errno("failed to write to %s", path);
+		}
 		strbuf_release(&sb);
 		return get_tempfile_path(temp);
 	}
@@ -319,9 +322,12 @@ void setup_alternate_shallow(struct lock_file *shallow_lock,
 				       LOCK_DIE_ON_ERROR);
 	check_shallow_file_for_update();
 	if (write_shallow_commits(&sb, 0, extra)) {
-		if (write_in_full(fd, sb.buf, sb.len) < 0)
-			die_errno("failed to write to %s",
-				  get_lock_file_path(shallow_lock));
+		if (write_in_full(fd, sb.buf, sb.len) < 0) {
+			int saved_errno = errno;
+			const char *path = get_lock_file_path(shallow_lock);
+			errno = saved_errno;
+			die_errno("failed to write to %s", path);
+		}
 		*alternate_shallow_file = get_lock_file_path(shallow_lock);
 	} else
 		/*
@@ -366,9 +372,12 @@ void prune_shallow(int show_only)
 				       LOCK_DIE_ON_ERROR);
 	check_shallow_file_for_update();
 	if (write_shallow_commits_1(&sb, 0, NULL, SEEN_ONLY)) {
-		if (write_in_full(fd, sb.buf, sb.len) < 0)
-			die_errno("failed to write to %s",
-				  get_lock_file_path(&shallow_lock));
+		if (write_in_full(fd, sb.buf, sb.len) < 0) {
+			int saved_errno = errno;
+			const char *path = get_lock_file_path(&shallow_lock);
+			errno = saved_errno;
+			die_errno("failed to write to %s", path);
+		}
 		commit_lock_file(&shallow_lock);
 	} else {
 		unlink(git_path_shallow());

-- 
2.16.1.205.g271f633410

-- 8< --
Previous: Junio C HamanoNext: Eric Sunshine
Message 21 of 40 in “Add "git rebase --show-patch"”
  1. 0/2 Add "git rebase --show-patch"Nguyễn Thái Ngọc Duy, Jan 26, 2018
  2. 1/2 am: add --show-patchNguyễn Thái Ngọc Duy, Jan 26, 2018
  3. 2/2 rebase: add --show-patchNguyễn Thái Ngọc Duy, Jan 26, 2018
  4. Phillip WoodJan 26, 2018
  5. Duy NguyenJan 26, 2018
  6. Phillip WoodJan 30, 2018
  7. Eric SunshineJan 26, 2018
  8. Duy NguyenJan 27, 2018
  9. Tim LandscheidtJan 26, 2018
  10. Duy NguyenJan 27, 2018
  11. Ævar Arnfjörð BjarmasonJan 27, 2018
  12. Johannes SchindelinJan 29, 2018
  13. Duy NguyenJan 30, 2018
  14. Johannes SchindelinJan 30, 2018
  15. Junio C HamanoJan 30, 2018
  16. Tim LandscheidtFeb 1, 2018
  17. 0/3 Add "git rebase --show-patch"Nguyễn Thái Ngọc Duy, Jan 31, 2018
  18. 1/3 am: add --show-current-patchNguyễn Thái Ngọc Duy, Jan 31, 2018
  19. Eric SunshineJan 31, 2018
  20. Junio C HamanoJan 31, 2018
  21. Duy NguyenFeb 2, 2018
  22. Eric SunshineFeb 2, 2018
  23. Duy NguyenFeb 2, 2018
  24. Junio C HamanoFeb 2, 2018
  25. gitignore.txt: elaborate shell glob syntaxNguyễn Thái Ngọc Duy, Jan 31, 2018
  26. Junio C HamanoJan 31, 2018
  27. Duy NguyenFeb 1, 2018
  28. 2/3 rebase: add --show-current-patchNguyễn Thái Ngọc Duy, Jan 31, 2018
  29. 3/3 rebase: introduce and use pseudo-ref ORIG_COMMITNguyễn Thái Ngọc Duy, Jan 31, 2018
  30. Junio C HamanoJan 31, 2018
  31. Duy NguyenFeb 1, 2018
  32. Junio C HamanoFeb 2, 2018
  33. Phillip WoodFeb 1, 2018
  34. 0/3 Add "git rebase --show-current-patch"Nguyễn Thái Ngọc Duy, Feb 11, 2018
  35. 1/3 am: add --show-current-patchNguyễn Thái Ngọc Duy, Feb 11, 2018
  36. 2/3 rebase: add --show-current-patchNguyễn Thái Ngọc Duy, Feb 11, 2018
  37. 3/3 rebase: introduce and use pseudo-ref REBASE_HEADNguyễn Thái Ngọc Duy, Feb 11, 2018
  38. Junio C HamanoFeb 22, 2018
  39. Tim LandscheidtFeb 22, 2018
  40. Duy NguyenFeb 23, 2018

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.