Re: [PATCH v2] gettext.c: only work around the vsnprintf bug on glibc < 2.17

From: Duy Nguyen <pclouds@gmail.com>
Date: Dec 1, 2013, 01:33 UTC
Message-ID: <CACsJy8BdFst0ZRpvMwUxby88YMJdiY4h386kTVP57h6XfgLLgg@mail.gmail.com>
In-Reply-To: <529A6E48.4050001@web.de>

On Sun, Dec 1, 2013 at 6:01 AM, Torsten Bögershausen <tboegi@web.de> wrote:

Show 8 quoted lines

> On 2013-11-30 13.01, Nguyễn Thái Ngọc Duy wrote:
>> Bug 6530 [1] causes "git show v0.99.6~1" to fail with error "your
> causes or caused (as we have a work around?)
>> vsnprintf is broken". The workaround avoids that, but it corrupts
>> system error messages in non-C locales.
> [snip]
>> The bug in glibc has been fixed since 2.17. If git is built with glibc, it can
>                 ^^^^^^ (Should we name glibc ?)

No, probably leftover from editing.

Show 8 quoted lines

> [snip]
>> -     setlocale(LC_MESSAGES, "");
>> -     init_gettext_charset("git");
>> +     setlocale(vsnprintf_broken ? LC_MESSAGES : LC_ALL, "");
> 1) One thing I don't understand: Why do we need to set LC_ALL ?
> The old patch didn't do it, or what do I miss ?
> See https://wiki.debian.org/Locale :
> Using LC_ALL is strongly discouraged as it overrides everything. Please use it only when testing and never set it in a startup file.

I'm fine with changing it back to LC_MESSAGES+LC_TYPE. For the record, all gtk+ apps do setlocale(LC_ALL, "");

Show 28 quoted lines

> 2) I stole the code partly from here:
>    http://sourceware.org/bugzilla/show_bug.cgi?id=6530
> ----------------------
> #include <stdio.h>
> #include <locale.h>
> #include <gnu/libc-version.h>
>
> #define STR "²éÄ¾ÂíÉ±²¡¶¾£¬ÖÜºèµtÄúµÄ360²»×¨Òµ£¡"
>
> int main(void) {
>         char buf[200];
>         setlocale(LC_ALL, "");
>                                 printf("gnu_glibc_version()=%s\n",  gnu_get_libc_version());
>         printf("ret(snprintf)=%d\n", snprintf(buf, 150, "%.50s", STR));
>         return 0;
> }
>
> ----------------------
> Then I run it on different machines:
>
> gnu_glibc_version()=2.11.3 /* Ubuntu 10.4, no updates */
> gnu_glibc_version()=2.11.3 /* Debian Squeze  ?*/
> gnu_glibc_version()=2.13 /* Debian Wheezy */
> ret(snprintf)=50 /* All the 3 above */
> -------------
> So could it be that libc is patched in Debian/Ubuntu, and we
> can do a runtime check (rather than looking at the version number),
> similar to the code above ?

Good idea. Now I need to install 2.16 for reproducing it :(

Show 12 quoted lines

> ------------
>
> 3) The patch didn't break anything here (Debian, Mac OS).
>
> 4) Could it be good to have a test case ? Is t0204 good for inspiration ?
>
> 5) I can do more testing if needed.
>
> /Torsten
>
>
>

-- 
Duy