Re: [PATCH v2 1/3] am: add --show-current-patch
- From
Duy Nguyen <pclouds@gmail.com>
- Date
- Feb 2, 2018, 09:53 UTC
- Message-ID
- <CACsJy8Cg35sqeEtmLJo2vrHvB4fD0uzZwYubgXOcZxUHMwcpqQ@mail.gmail.com>
- In-Reply-To
- <CAPig+cQKMxwadf9aGyC5ESa-vxDy9PzrYo+m+JaVQ3S=12PyQQ@mail.gmail.com>
On Fri, Feb 2, 2018 at 4:46 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
Show 31 quoted lines
> 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);That would be best. Unfortunately we have HAVE_VARIADIC_MACROS so we need to deal with no variadic macro support too.
-- Duy