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

Eric Sunshine [off-list ref] writes:

On Wed, Jan 31, 2018 at 4:30 AM, Nguyễn Thái Ngọc Duy [off-list ref] wrote:

quoted

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 <redacted>
---

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.  After coming up with the pathname to the current patch file,
we are going to exit without ever calling am_path(), or underlying
get_pathname() via mkpath(), again before exiting anyway, so perhaps
it is sufficient to just do an am_path() just once upfront, feed it
to strbuf_read_file() and also to die_errno().