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

[Eric Sunshine <sunshine@sunshineco.com>]

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?

> +       setup_pager();
> +       write_in_full(1, sb.buf, sb.len);
> +       strbuf_release(&sb);
> +       return 0;
> +}