On Tue, Jul 3, 2012 at 10:45 AM, Jeff King <peff@xxxxxxxx> wrote:
> On Tue, Jul 03, 2012 at 12:43:42AM +0200, Andreas Schwab wrote:
>
>> Jeff King <peff@xxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html