From: Nguyen Thai Ngoc Duy Date: Tue, 03 Jul 2012 04:25:16 GMT Subject: Re: bug: "fatal: cannot pread pack file", version 1.7.5.4 Message-ID: In-Reply-To: <20120703032345.GA7143@sigill.intra.peff.net> On Tue, Jul 3, 2012 at 10:45 AM, Jeff King wrote: > On Tue, Jul 03, 2012 at 12:43:42AM +0200, Andreas Schwab wrote: > >> Jeff King writes: >> >> > It's very odd for pread to report ENOENT (since it is always operating >> > on an already-opened file descriptor). >> >> It doesn't, but gettext will clobber errno: >> >>               n = pread(pack_fd, inbuf, n, from); >>               if (n < 0) >>                       die_errno(_("cannot pread pack file")); >> >> There is nothing that saves errno.  This isn't limited to i18n though, >> any function call in the arguments may potentially clobber errno. > > That's horribly lame of gettext. I don't expect arbitrary functions to > save errno, but when the entire purpose of a function is to be a > non-intrusive wrapper to massage messages to the user, it seems kind of > evil to overwrite errno. Isn't the whole point of calling it "_" that > you don't want to or have to notice it? Agreed. > Can we do something like this to get around it? > > diff --git a/gettext.h b/gettext.h > index 57ba8bb..b7c3ae5 100644 > --- a/gettext.h > +++ b/gettext.h > @@ -44,7 +44,10 @@ extern int use_gettext_poison(void); > >  static inline FORMAT_PRESERVING(1) const char *_(const char *msgid) >  { > -       return use_gettext_poison() ? "# GETTEXT POISON #" : gettext(msgid); > +       int saved_errno = errno; > +       const char *r = use_gettext_poison() ? "# GETTEXT POISON #" : gettext(msgid); > +       errno = saved_errno; > +       return r; >  } > >  static inline FORMAT_PRESERVING(1) FORMAT_PRESERVING(2) The last line belongs to Q_(), which needs the same treatment. -- Duy