Thread (4 messages) flat view 4 messages, 3 authors, 2018-02-02

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

From: Eric Sunshine <hidden>
Date: 2018-02-02 09:46:35

Possibly related (same subject, not in this thread)

On Fri, Feb 2, 2018 at 4:25 AM, Duy Nguyen [off-list ref] wrote:
On Wed, Jan 31, 2018 at 02:59:32PM -0800, Junio C Hamano wrote:
quoted
Eric Sunshine [off-list ref] writes:
quoted
On Wed, Jan 31, 2018 at 4:30 AM, Nguyễn Thái Ngọc Duy [off-list ref] wrote:
quoted
+       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);
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help