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

From: Eric Sunshine <sunshine@sunshineco.com>
To: Duy Nguyen <pclouds@gmail.com>
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Git List" <git@vger.kernel.org>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"Tim Landscheidt" <tim@tim-landscheidt.de>
Subject: Re: [PATCH v2 1/3] am: add --show-current-patch
Date: Fri, 2 Feb 2018 04:46:27 -0500	[thread overview]
Message-ID: <CAPig+cQKMxwadf9aGyC5ESa-vxDy9PzrYo+m+JaVQ3S=12PyQQ@mail.gmail.com> (raw)
In-Reply-To: <20180202092511.GA28946@ash>

On Fri, Feb 2, 2018 at 4:25 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Wed, Jan 31, 2018 at 02:59:32PM -0800, Junio C Hamano wrote:
>> 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:
>> >> +       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?
>
> -       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);

Rather than worrying about catching these at review time, I had been
thinking about a solution which automates it using variadic macros.
Something like:

    #define die_errno(...) do { \
        int saved_errno_ = errno; \
        die_errno_(saved_errno_, __VA_ARGS__); \
        } while (0);