{"thread":{"id":"35441","subject":"[PATCH] gettext.c: only work around the vsnprintf bug on glibc < 2.17","startedAt":"2013-11-30T01:51:31Z","lastAt":"2013-12-02T09:00:14Z","messageCount":12,"participants":["Nguyễn Thái Ngọc Duy","Andreas Schwab","Torsten Bögershausen","Duy Nguyen","Trần Ngọc Quân"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"231289","messageId":"1385776291-21006-1-git-send-email-pclouds@gmail.com","threadId":"35441","inReplyTo":null,"subject":"[PATCH] gettext.c: only work around the vsnprintf bug on glibc < 2.17","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-11-30T01:51:31Z","receivedAt":"2013-11-30T01:51:31Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Bug 6530 [1] causes \"git show v0.99.6~1\" to fail with error \"your\nvsnprintf is broken\". The workaround avoids that, but it corrupts\nsystem error messages in non-C locales.\n\nThe bug has been fixed since 2.17. If git is built with glibc, it can\nknow running libc version with gnu_get_libc_version() and avoid the\nworkaround on fixed versions. As a side effect, the workaround is\ndropped for all non-glibc systems.\n\nTested on Gentoo Linux, glibc 2.17, amd64.\n\n[1] http://sourceware.org/bugzilla/show_bug.cgi?id=6530\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n We could even dlopen and look for gnu_get_libc_version at runtime and\n drop USE_GLIBC define. But there may be other problems with dl* on\n other platforms..\n\n Somebody should test for the other two \"USE_GLIBC = YesPlease\" I\n added. I don't have Debian/FreeBSD nor any non-Linux GNU systems.\n\n Makefile         |  5 +++++\n config.mak.uname |  3 +++\n gettext.c        | 34 ++++++++++++++++++++++++++++------\n 3 files changed, 36 insertions(+), 6 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex af847f8..8df6d6d 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -66,6 +66,8 @@ all::\n # Define LIBC_CONTAINS_LIBINTL if your gettext implementation doesn't\n # need -lintl when linking.\n #\n+# Define USE_GLIBC if your libc version is glibc >= 2.1.\n+#\n # Define NO_MSGFMT_EXTENDED_OPTIONS if your implementation of msgfmt\n # doesn't support GNU extensions like --check and --statistics\n #\n@@ -1203,6 +1205,9 @@ ifndef NO_GETTEXT\n ifndef LIBC_CONTAINS_LIBINTL\n \tEXTLIBS += -lintl\n endif\n+ifdef USE_GLIBC\n+\tBASIC_CFLAGS += -DUSE_GLIBC\n+endif\n endif\n ifdef NEEDS_SOCKET\n \tEXTLIBS += -lsocket\ndiff --git a/config.mak.uname b/config.mak.uname\nindex 82d549e..ffb01e0 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -33,6 +33,7 @@ ifeq ($(uname_S),Linux)\n \tHAVE_PATHS_H = YesPlease\n \tLIBC_CONTAINS_LIBINTL = YesPlease\n \tHAVE_DEV_TTY = YesPlease\n+\tUSE_GLIBC = YesPlease\n endif\n ifeq ($(uname_S),GNU/kFreeBSD)\n \tNO_STRLCPY = YesPlease\n@@ -40,6 +41,7 @@ ifeq ($(uname_S),GNU/kFreeBSD)\n \tHAVE_PATHS_H = YesPlease\n \tDIR_HAS_BSD_GROUP_SEMANTICS = YesPlease\n \tLIBC_CONTAINS_LIBINTL = YesPlease\n+\tUSE_GLIBC = YesPlease\n endif\n ifeq ($(uname_S),UnixWare)\n \tCC = cc\n@@ -236,6 +238,7 @@ ifeq ($(uname_S),GNU)\n \tNO_MKSTEMPS = YesPlease\n \tHAVE_PATHS_H = YesPlease\n \tLIBC_CONTAINS_LIBINTL = YesPlease\n+\tUSE_GLIBC = YesPlease\n endif\n ifeq ($(uname_S),IRIX)\n \tNO_SETENV = YesPlease\ndiff --git a/gettext.c b/gettext.c\nindex 71e9545..91e679d 100644\n--- a/gettext.c\n+++ b/gettext.c\n@@ -18,6 +18,13 @@\n #\tendif\n #endif\n \n+#ifdef USE_GLIBC\n+#ifndef _GNU_SOURCE\n+#define _GNU_SOURCE\n+#endif\n+#include <gnu/libc-version.h>\n+#endif\n+\n #ifdef GETTEXT_POISON\n int use_gettext_poison(void)\n {\n@@ -30,6 +37,7 @@ int use_gettext_poison(void)\n \n #ifndef NO_GETTEXT\n static const char *charset;\n+static int vsnprintf_workaround;\n static void init_gettext_charset(const char *domain)\n {\n \t/*\n@@ -99,9 +107,7 @@ static void init_gettext_charset(const char *domain)\n \t   $ LANGUAGE= LANG=de_DE.utf8 ./test\n \t   test: Kein passendes Ger?t gefunden\n \n-\t   In the long term we should probably see about getting that\n-\t   vsnprintf bug in glibc fixed, and audit our code so it won't\n-\t   fall apart under a non-C locale.\n+\t   The vsnprintf bug has been fixed since 2.17.\n \n \t   Then we could simply set LC_CTYPE from the environment, which would\n \t   make things like the external perror(3) messages work.\n@@ -112,20 +118,36 @@ static void init_gettext_charset(const char *domain)\n \t   1. http://sourceware.org/bugzilla/show_bug.cgi?id=6530\n \t   2. E.g. \"Content-Type: text/plain; charset=UTF-8\\n\" in po/is.po\n \t*/\n-\tsetlocale(LC_CTYPE, \"\");\n+\tif (vsnprintf_workaround)\n+\t\tsetlocale(LC_CTYPE, \"\");\n \tcharset = locale_charset();\n \tbind_textdomain_codeset(domain, charset);\n-\tsetlocale(LC_CTYPE, \"C\");\n+\tif (vsnprintf_workaround)\n+\t\tsetlocale(LC_CTYPE, \"C\");\n }\n \n void git_setup_gettext(void)\n {\n \tconst char *podir = getenv(\"GIT_TEXTDOMAINDIR\");\n \n+#ifdef USE_GLIBC\n+\tint major, minor;\n+\tconst char *version = gnu_get_libc_version();\n+\n+\tif (version && sscanf(version, \"%d.%d\", &major, &minor) == 2 &&\n+\t    (major > 2 || (major == 2 && minor >= 17)))\n+\t\tvsnprintf_workaround = 0;\n+\telse\n+\t\tvsnprintf_workaround = 1;\n+#endif\n+\n \tif (!podir)\n \t\tpodir = GIT_LOCALE_PATH;\n \tbindtextdomain(\"git\", podir);\n-\tsetlocale(LC_MESSAGES, \"\");\n+\tif (vsnprintf_workaround)\n+\t\tsetlocale(LC_MESSAGES, \"\");\n+\telse\n+\t\tsetlocale(LC_ALL, \"\");\n \tinit_gettext_charset(\"git\");\n \ttextdomain(\"git\");\n }\n-- \n1.8.2.83.gc99314b\n"},{"id":"231294","messageId":"m2y546b4ka.fsf@linux-m68k.org","threadId":"35441","inReplyTo":"1385776291-21006-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] gettext.c: only work around the vsnprintf bug on glibc < 2.17","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2013-11-30T09:51:33Z","receivedAt":"2013-11-30T09:51:33Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:\n\n> diff --git a/gettext.c b/gettext.c\n> index 71e9545..91e679d 100644\n> --- a/gettext.c\n> +++ b/gettext.c\n> @@ -18,6 +18,13 @@\n>  #\tendif\n>  #endif\n>  \n> +#ifdef USE_GLIBC\n> +#ifndef _GNU_SOURCE\n> +#define _GNU_SOURCE\n> +#endif\n\nDefining a feature test macro after any system header is included is\nundefined.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"231299","messageId":"1385812884-23776-1-git-send-email-pclouds@gmail.com","threadId":"35441","inReplyTo":"1385776291-21006-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2] gettext.c: only work around the vsnprintf bug on glibc < 2.17","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-11-30T12:01:24Z","receivedAt":"2013-11-30T12:01:24Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Bug 6530 [1] causes \"git show v0.99.6~1\" to fail with error \"your\nvsnprintf is broken\". The workaround avoids that, but it corrupts\nsystem error messages in non-C locales.\n\nThe bug has been fixed since 2.17. If git is built with glibc, it can\nknow running libc version with gnu_get_libc_version() and avoid the\nworkaround on fixed versions. The workaround is also dropped for all\nnon-glibc systems.\n\nTested on Gentoo Linux, glibc 2.17, amd64.\n\n[1] http://sourceware.org/bugzilla/show_bug.cgi?id=6530\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n v2 removes USE_GLIBC and lets git-compat-util.h do the detection\n\n gettext.c         | 24 ++++++++++++++++--------\n git-compat-util.h | 11 +++++++++++\n 2 files changed, 27 insertions(+), 8 deletions(-)\n\ndiff --git a/gettext.c b/gettext.c\nindex 71e9545..772ab92 100644\n--- a/gettext.c\n+++ b/gettext.c\n@@ -30,7 +30,7 @@ int use_gettext_poison(void)\n \n #ifndef NO_GETTEXT\n static const char *charset;\n-static void init_gettext_charset(const char *domain)\n+static void init_gettext_charset(const char *domain, int vsnprintf_broken)\n {\n \t/*\n \t   This trick arranges for messages to be emitted in the user's\n@@ -99,9 +99,7 @@ static void init_gettext_charset(const char *domain)\n \t   $ LANGUAGE= LANG=de_DE.utf8 ./test\n \t   test: Kein passendes Ger?t gefunden\n \n-\t   In the long term we should probably see about getting that\n-\t   vsnprintf bug in glibc fixed, and audit our code so it won't\n-\t   fall apart under a non-C locale.\n+\t   The vsnprintf bug has been fixed since 2.17.\n \n \t   Then we could simply set LC_CTYPE from the environment, which would\n \t   make things like the external perror(3) messages work.\n@@ -112,21 +110,31 @@ static void init_gettext_charset(const char *domain)\n \t   1. http://sourceware.org/bugzilla/show_bug.cgi?id=6530\n \t   2. E.g. \"Content-Type: text/plain; charset=UTF-8\\n\" in po/is.po\n \t*/\n-\tsetlocale(LC_CTYPE, \"\");\n+\tif (vsnprintf_broken)\n+\t\tsetlocale(LC_CTYPE, \"\");\n \tcharset = locale_charset();\n \tbind_textdomain_codeset(domain, charset);\n-\tsetlocale(LC_CTYPE, \"C\");\n+\tif (vsnprintf_broken)\n+\t\tsetlocale(LC_CTYPE, \"C\");\n }\n \n void git_setup_gettext(void)\n {\n \tconst char *podir = getenv(\"GIT_TEXTDOMAINDIR\");\n+\tconst char *version = glibc_version();\n+\tint major, minor, vsnprintf_broken;\n+\n+\tif (version && sscanf(version, \"%d.%d\", &major, &minor) == 2 &&\n+\t    (major > 2 || (major == 2 && minor >= 17)))\n+\t\tvsnprintf_broken = 0;\n+\telse\n+\t\tvsnprintf_broken = 1;\n \n \tif (!podir)\n \t\tpodir = GIT_LOCALE_PATH;\n \tbindtextdomain(\"git\", podir);\n-\tsetlocale(LC_MESSAGES, \"\");\n-\tinit_gettext_charset(\"git\");\n+\tsetlocale(vsnprintf_broken ? LC_MESSAGES : LC_ALL, \"\");\n+\tinit_gettext_charset(\"git\", vsnprintf_broken);\n \ttextdomain(\"git\");\n }\n \ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 7776f12..967f452 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -488,11 +488,22 @@ extern int git_vsnprintf(char *str, size_t maxsize,\n \n #ifdef __GLIBC_PREREQ\n #if __GLIBC_PREREQ(2, 1)\n+#include <gnu/libc-version.h>\n+#define glibc_version() gnu_get_libc_version()\n #define HAVE_STRCHRNUL\n #define HAVE_MEMPCPY\n #endif\n #endif\n \n+#ifndef glibc_version\n+#ifdef __GNU_LIBRARY__\n+#define glibc_version() NULL\n+#else\n+/* non-glibc platforms, see git_setup_gettext() for \"2.17\" */\n+#define glibc_version() \"2.17\"\n+#endif\n+#endif\n+\n #ifndef HAVE_STRCHRNUL\n #define strchrnul gitstrchrnul\n static inline char *gitstrchrnul(const char *s, int c)\n-- \n1.8.2.83.gc99314b\n"},{"id":"231316","messageId":"529A6E48.4050001@web.de","threadId":"35441","inReplyTo":"1385812884-23776-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v2] gettext.c: only work around the vsnprintf bug on glibc < 2.17","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2013-11-30T23:01:28Z","receivedAt":"2013-11-30T23:01:28Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 2013-11-30 13.01, Nguyễn Thái Ngọc Duy wrote:\n> Bug 6530 [1] causes \"git show v0.99.6~1\" to fail with error \"your\ncauses or caused (as we have a work around?)\n> vsnprintf is broken\". The workaround avoids that, but it corrupts\n> system error messages in non-C locales.\n[snip]\n> The bug in glibc has been fixed since 2.17. If git is built with glibc, it can\n                ^^^^^^ (Should we name glibc ?)\n[snip]\n> -\tsetlocale(LC_MESSAGES, \"\");\n> -\tinit_gettext_charset(\"git\");\n> +\tsetlocale(vsnprintf_broken ? LC_MESSAGES : LC_ALL, \"\");\n1) One thing I don't understand: Why do we need to set LC_ALL ?\nThe old patch didn't do it, or what do I miss ?\nSee https://wiki.debian.org/Locale :\nUsing LC_ALL is strongly discouraged as it overrides everything. Please use it only when testing and never set it in a startup file.\n2) I stole the code partly from here:\n   http://sourceware.org/bugzilla/show_bug.cgi?id=6530\n----------------------\n#include <stdio.h>\n#include <locale.h>\n#include <gnu/libc-version.h>\n\n#define STR \"²éľÂíɱ²¡¶¾£¬ÖܺèµtÄúµÄ360²»×¨Òµ£¡\"\n\nint main(void) {\n        char buf[200];\n        setlocale(LC_ALL, \"\");\n                                printf(\"gnu_glibc_version()=%s\\n\",  gnu_get_libc_version());\n        printf(\"ret(snprintf)=%d\\n\", snprintf(buf, 150, \"%.50s\", STR));\n        return 0;\n}\n\n----------------------\nThen I run it on different machines:\n\ngnu_glibc_version()=2.11.3 /* Ubuntu 10.4, no updates */\ngnu_glibc_version()=2.11.3 /* Debian Squeze  ?*/\ngnu_glibc_version()=2.13 /* Debian Wheezy */\nret(snprintf)=50 /* All the 3 above */\n-------------\nSo could it be that libc is patched in Debian/Ubuntu, and we\ncan do a runtime check (rather than looking at the version number),\nsimilar to the code above ?\n------------\n\n3) The patch didn't break anything here (Debian, Mac OS).\n\n4) Could it be good to have a test case ? Is t0204 good for inspiration ?\n\n5) I can do more testing if needed.\n\n/Torsten\n"},{"id":"231317","messageId":"529A6F72.9030909@web.de","threadId":"35441","inReplyTo":"529A6E48.4050001@web.de","subject":"Re: [PATCH v2] gettext.c: only work around the vsnprintf bug on glibc < 2.17","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2013-11-30T23:06:26Z","receivedAt":"2013-11-30T23:06:26Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"> gnu_glibc_version()=2.11.3 /* Ubuntu 10.4, no updates */\nSorry, Copy-paste error:\n2.11.1\n"},{"id":"231319","messageId":"CACsJy8BdFst0ZRpvMwUxby88YMJdiY4h386kTVP57h6XfgLLgg@mail.gmail.com","threadId":"35441","inReplyTo":"529A6E48.4050001@web.de","subject":"Re: [PATCH v2] gettext.c: only work around the vsnprintf bug on glibc < 2.17","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-12-01T01:33:21Z","receivedAt":"2013-12-01T01:33:21Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Dec 1, 2013 at 6:01 AM, Torsten Bögershausen <tboegi@web.de> wrote:\n> On 2013-11-30 13.01, Nguyễn Thái Ngọc Duy wrote:\n>> Bug 6530 [1] causes \"git show v0.99.6~1\" to fail with error \"your\n> causes or caused (as we have a work around?)\n>> vsnprintf is broken\". The workaround avoids that, but it corrupts\n>> system error messages in non-C locales.\n> [snip]\n>> The bug in glibc has been fixed since 2.17. If git is built with glibc, it can\n>                 ^^^^^^ (Should we name glibc ?)\n\nNo, probably leftover from editing.\n\n> [snip]\n>> -     setlocale(LC_MESSAGES, \"\");\n>> -     init_gettext_charset(\"git\");\n>> +     setlocale(vsnprintf_broken ? LC_MESSAGES : LC_ALL, \"\");\n> 1) One thing I don't understand: Why do we need to set LC_ALL ?\n> The old patch didn't do it, or what do I miss ?\n> See https://wiki.debian.org/Locale :\n> Using LC_ALL is strongly discouraged as it overrides everything. Please use it only when testing and never set it in a startup file.\n\nI'm fine with changing it back to LC_MESSAGES+LC_TYPE. For the record,\nall gtk+ apps do setlocale(LC_ALL, \"\");\n\n> 2) I stole the code partly from here:\n>    http://sourceware.org/bugzilla/show_bug.cgi?id=6530\n> ----------------------\n> #include <stdio.h>\n> #include <locale.h>\n> #include <gnu/libc-version.h>\n>\n> #define STR \"²éľÂíɱ²¡¶¾£¬ÖܺèµtÄúµÄ360²»×¨Òµ£¡\"\n>\n> int main(void) {\n>         char buf[200];\n>         setlocale(LC_ALL, \"\");\n>                                 printf(\"gnu_glibc_version()=%s\\n\",  gnu_get_libc_version());\n>         printf(\"ret(snprintf)=%d\\n\", snprintf(buf, 150, \"%.50s\", STR));\n>         return 0;\n> }\n>\n> ----------------------\n> Then I run it on different machines:\n>\n> gnu_glibc_version()=2.11.3 /* Ubuntu 10.4, no updates */\n> gnu_glibc_version()=2.11.3 /* Debian Squeze  ?*/\n> gnu_glibc_version()=2.13 /* Debian Wheezy */\n> ret(snprintf)=50 /* All the 3 above */\n> -------------\n> So could it be that libc is patched in Debian/Ubuntu, and we\n> can do a runtime check (rather than looking at the version number),\n> similar to the code above ?\n\nGood idea. Now I need to install 2.16 for reproducing it :(\n\n> ------------\n>\n> 3) The patch didn't break anything here (Debian, Mac OS).\n>\n> 4) Could it be good to have a test case ? Is t0204 good for inspiration ?\n>\n> 5) I can do more testing if needed.\n>\n> /Torsten\n>\n>\n>\n\n\n\n-- \nDuy\n"},{"id":"231320","messageId":"1385865938-16392-1-git-send-email-pclouds@gmail.com","threadId":"35441","inReplyTo":"1385812884-23776-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v3] gettext.c: detect the vsnprintf bug at runtime","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-12-01T02:45:38Z","receivedAt":"2013-12-01T02:45:38Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Bug 6530 [1] in glibc causes \"git show v0.99.6~1\" to fail with error\n\"your vsnprintf is broken\". The workaround avoids that, but it\ncorrupts system error messages in non-C locales.\n\nThe bug has been fixed since 2.17. We could know running glibc version\nwith gnu_get_libc_version(). But version is not a sure way to detect\nthe bug because downstream may back port the fix to older versions. Do\na runtime test that immitates the call flow that leads to \"your\nvsnprintf is broken\". Only enable the workaround if the test fails.\n\nTested on Gentoo Linux, glibc 2.16.0 and 2.17, amd64.\n\n[1] http://sourceware.org/bugzilla/show_bug.cgi?id=6530\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n v3 goes with runtime test instead of version check.\n\n gettext.c | 19 +++++++++++++++----\n 1 file changed, 15 insertions(+), 4 deletions(-)\n\ndiff --git a/gettext.c b/gettext.c\nindex 71e9545..eed7c7f 100644\n--- a/gettext.c\n+++ b/gettext.c\n@@ -29,6 +29,17 @@ int use_gettext_poison(void)\n #endif\n \n #ifndef NO_GETTEXT\n+static int test_vsnprintf(const char *fmt, ...)\n+{\n+\tchar buf[26];\n+\tint ret;\n+\tva_list ap;\n+\tva_start(ap, fmt);\n+\tret = vsnprintf(buf, sizeof(buf), fmt, ap);\n+\tva_end(ap);\n+\treturn ret;\n+}\n+\n static const char *charset;\n static void init_gettext_charset(const char *domain)\n {\n@@ -99,9 +110,7 @@ static void init_gettext_charset(const char *domain)\n \t   $ LANGUAGE= LANG=de_DE.utf8 ./test\n \t   test: Kein passendes Ger?t gefunden\n \n-\t   In the long term we should probably see about getting that\n-\t   vsnprintf bug in glibc fixed, and audit our code so it won't\n-\t   fall apart under a non-C locale.\n+\t   The vsnprintf bug has been fixed since glibc 2.17.\n \n \t   Then we could simply set LC_CTYPE from the environment, which would\n \t   make things like the external perror(3) messages work.\n@@ -115,7 +124,9 @@ static void init_gettext_charset(const char *domain)\n \tsetlocale(LC_CTYPE, \"\");\n \tcharset = locale_charset();\n \tbind_textdomain_codeset(domain, charset);\n-\tsetlocale(LC_CTYPE, \"C\");\n+\t/* the string is taken from v0.99.6~1 */\n+\tif (test_vsnprintf(\"%.*s\", 13, \"David_K\\345gedal\") < 0)\n+\t\tsetlocale(LC_CTYPE, \"C\");\n }\n \n void git_setup_gettext(void)\n-- \n1.8.2.83.gc99314b\n"},{"id":"231347","messageId":"529BD4E1.8040408@gmail.com","threadId":"35441","inReplyTo":"1385865938-16392-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v3] gettext.c: detect the vsnprintf bug at runtime","fromName":"Trần Ngọc Quân","fromEmail":"vnwildman@gmail.com","sentAt":"2013-12-02T00:31:29Z","receivedAt":"2013-12-02T00:31:29Z","isPatch":true,"sender":{"key":"vnwildman@gmail.com","avatar":"https://avatars.githubusercontent.com/u/508758?v=4"},"body":"On 01/12/2013 09:45, Nguyễn Thái Ngọc Duy wrote:\n> Bug 6530 [1] in glibc causes \"git show v0.99.6~1\" to fail with error\n> \"your vsnprintf is broken\". The workaround avoids that, but it\n> corrupts system error messages in non-C locales.\n>\n> The bug has been fixed since 2.17. We could know running glibc version\n> with gnu_get_libc_version(). But version is not a sure way to detect\n> the bug because downstream may back port the fix to older versions. Do\n> a runtime test that immitates the call flow that leads to \"your\n> vsnprintf is broken\". Only enable the workaround if the test fails.\n>\n> Tested on Gentoo Linux, glibc 2.16.0 and 2.17, amd64.\n>\n> [1] http://sourceware.org/bugzilla/show_bug.cgi?id=6530\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> ---\n>  v3 goes with runtime test instead of version check.\n>\n>  gettext.c | 19 +++++++++++++++----\n>  1 file changed, 15 insertions(+), 4 deletions(-)\n>\n> diff --git a/gettext.c b/gettext.c\n> index 71e9545..eed7c7f 100644\n> --- a/gettext.c\n> +++ b/gettext.c\n> @@ -29,6 +29,17 @@ int use_gettext_poison(void)\n>  #endif\n>  \n>  #ifndef NO_GETTEXT\n> +static int test_vsnprintf(const char *fmt, ...)\n> +{\n> +\tchar buf[26];\n> +\tint ret;\n> +\tva_list ap;\n> +\tva_start(ap, fmt);\n> +\tret = vsnprintf(buf, sizeof(buf), fmt, ap);\n> +\tva_end(ap);\n> +\treturn ret;\n> +}\n> +\nthis function alway run each time we run git commad while libc is\nstatic. It is waste.\n> +\t/* the string is taken from v0.99.6~1 */\n> +\tif (test_vsnprintf(\"%.*s\", 13, \"David_K\\345gedal\") < 0)\n> +\t\tsetlocale(LC_CTYPE, \"C\");\n>  }\n>  \n>  void git_setup_gettext(void)\n\nI suggest use C preprocessor instead. The person who complete git (make debian, rpm etc. package) decide  enable it or not (disable by default). Most of people use git from distribution instead of complete it from source.\n\n#ifndef VSNPRINTF_OK\n\tsetlocale(LC_CTYPE, \"C\");\n#endif\n\n\n-- \nTrần Ngọc Quân.\n"},{"id":"231350","messageId":"CACsJy8B5aBGnm=04y60W1XVovHVryP1Co0_mmgV0f7Ox13aGjw@mail.gmail.com","threadId":"35441","inReplyTo":"529BD4E1.8040408@gmail.com","subject":"Re: [PATCH v3] gettext.c: detect the vsnprintf bug at runtime","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-12-02T05:57:36Z","receivedAt":"2013-12-02T05:57:36Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Dec 2, 2013 at 7:31 AM, Trần Ngọc Quân <vnwildman@gmail.com> wrote:\n>> --- a/gettext.c\n>> +++ b/gettext.c\n>> @@ -29,6 +29,17 @@ int use_gettext_poison(void)\n>>  #endif\n>>\n>>  #ifndef NO_GETTEXT\n>> +static int test_vsnprintf(const char *fmt, ...)\n>> +{\n>> +     char buf[26];\n>> +     int ret;\n>> +     va_list ap;\n>> +     va_start(ap, fmt);\n>> +     ret = vsnprintf(buf, sizeof(buf), fmt, ap);\n>> +     va_end(ap);\n>> +     return ret;\n>> +}\n>> +\n> this function alway run each time we run git commad while libc is\n> static. It is waste.\n>> +     /* the string is taken from v0.99.6~1 */\n>> +     if (test_vsnprintf(\"%.*s\", 13, \"David_K\\345gedal\") < 0)\n>> +             setlocale(LC_CTYPE, \"C\");\n>>  }\n>>\n>>  void git_setup_gettext(void)\n>\n> I suggest use C preprocessor instead. The person who complete git (make debian, rpm etc. package) decide  enable it or not (disable by default). Most of people use git from distribution instead of complete it from source.\n>\n> #ifndef VSNPRINTF_OK\n>         setlocale(LC_CTYPE, \"C\");\n> #endif\n>\n\nA single vsnprintf is cheap enough that I would not worry about\nperformance impact. Given a choice between this and distro\nmaintainers, some of them do check release notes, some not so much,\nI'd rather go with this.\n-- \nDuy\n"},{"id":"231353","messageId":"529C3987.7070708@gmail.com","threadId":"35441","inReplyTo":"CACsJy8B5aBGnm=04y60W1XVovHVryP1Co0_mmgV0f7Ox13aGjw@mail.gmail.com","subject":"Re: [PATCH v3] gettext.c: detect the vsnprintf bug at runtime","fromName":"Trần Ngọc Quân","fromEmail":"vnwildman@gmail.com","sentAt":"2013-12-02T07:40:55Z","receivedAt":"2013-12-02T07:40:55Z","isPatch":true,"sender":{"key":"vnwildman@gmail.com","avatar":"https://avatars.githubusercontent.com/u/508758?v=4"},"body":"On 02/12/2013 12:57, Duy Nguyen wrote:\n>> I suggest use C preprocessor instead. The person who complete git (make debian, rpm etc. package) decide  enable it or not (disable by default). Most of people use git from distribution instead of complete it from source.\n>>\n>> #ifndef VSNPRINTF_OK\n>>         setlocale(LC_CTYPE, \"C\");\n>> #endif\n>>\n> A single vsnprintf is cheap enough that I would not worry about\n> performance impact. Given a choice between this and distro\n> maintainers, some of them do check release notes, some not so much,\n> I'd rather go with this.\nWe can set this macro automatically  by using autoconf.\nAdd following code in configure.ac\n\n\nAC_LANG_CONFTEST(\n[AC_LANG_PROGRAM([[\n#include <stdio.h>\n#include <locale.h>\n#include <gnu/libc-version.h>\n\n#define STR \"David_K\\345gedal\"\n]],[[\n    char buf[20];\n    setlocale(LC_ALL, \"en_US.UTF-8\");\n    if (snprintf(buf, 13, \"%.13s\", STR) < 0){\n        printf(\"0\");\n    }else{\n        printf(\"1\");\n    }\n]])])\ngcc -o conftest conftest.c\nAC_DEFINE([VSNPRINTF_OK], [m4_esyscmd([./conftest])], [Enable l10n libc\nif vnsprintf OK])\n\nYou can change c code here!\n\n-- \nTrần Ngọc Quân.\n"},{"id":"231355","messageId":"529C49B2.6050402@gmail.com","threadId":"35441","inReplyTo":"529C3987.7070708@gmail.com","subject":"Re: [PATCH v3] gettext.c: detect the vsnprintf bug at runtime","fromName":"Trần Ngọc Quân","fromEmail":"vnwildman@gmail.com","sentAt":"2013-12-02T08:49:54Z","receivedAt":"2013-12-02T08:49:54Z","isPatch":true,"sender":{"key":"vnwildman@gmail.com","avatar":"https://avatars.githubusercontent.com/u/508758?v=4"},"body":"On 02/12/2013 14:40, Trần Ngọc Quân wrote:\n> On 02/12/2013 12:57, Duy Nguyen wrote:\n>>> I suggest use C preprocessor instead. The person who complete git (make debian, rpm etc. package) decide  enable it or not (disable by default). Most of people use git from distribution instead of complete it from source.\n>>>\n>>> #ifndef VSNPRINTF_OK\n>>>         setlocale(LC_CTYPE, \"C\");\n>>> #endif\n>>>\n>> A single vsnprintf is cheap enough that I would not worry about\n>> performance impact. Given a choice between this and distro\n>> maintainers, some of them do check release notes, some not so much,\n>> I'd rather go with this.\n> We can set this macro automatically  by using autoconf.\n> Add following code in configure.ac\n>\n>\n> AC_LANG_CONFTEST(\n> [AC_LANG_PROGRAM([[\n> #include <stdio.h>\n> #include <locale.h>\n> #include <gnu/libc-version.h>\n>\n> #define STR \"David_K\\345gedal\"\n> ]],[[\n>     char buf[20];\n>     setlocale(LC_ALL, \"en_US.UTF-8\");\n>     if (snprintf(buf, 13, \"%.13s\", STR) < 0){\n>         printf(\"0\");\n>     }else{\n>         printf(\"1\");\n>     }\n> ]])])\n> gcc -o conftest conftest.c\n> AC_DEFINE([VSNPRINTF_OK], [m4_esyscmd([./conftest])], [Enable l10n libc\n> if vnsprintf OK])\n>\n> You can change c code here!\n>\nSorry I'm wrong,  m4_esyscmd don't work as I spect\nChange to:\nAC_LANG_CONFTEST(\n[AC_LANG_PROGRAM([[\n#include <stdio.h>\n#include <locale.h>\n#include <gnu/libc-version.h>\n\n#define STR \"David_K\\345gedal\"\n]],[[\n    char buf[20];\n    setlocale(LC_CTYPE, \"en_US.UTF-8\");\n    if (snprintf(buf, 13, \"%.13s\", STR) < 0){\n        printf(\"0\");\n    }else{\n        printf(\"1\");\n    }\n]])])\ngcc -o conftest conftest.c\nVSNPRINTF=$(./conftest)\nAC_DEFINE_UNQUOTED([VSNPRINTF_OK], [ $VSNPRINTF ], [Enable libc if\nvnsprintf OK])\n\ndon't missing run this:\n$ autoconf && autoheader\n\nMy os (ubuntu 12.04) don't pass this test (libc6:i386     2.15-0ubuntu)\n\n$ grep VSNPRINTF_OK  config.h\n#define VSNPRINTF_OK 0\n\n-- \nTrần Ngọc Quân.\n"},{"id":"231356","messageId":"CACsJy8BqzH7G4yyXMnhgA6kH25eipt9ubqL1S2OdM-ke8GnvJA@mail.gmail.com","threadId":"35441","inReplyTo":"529C3987.7070708@gmail.com","subject":"Re: [PATCH v3] gettext.c: detect the vsnprintf bug at runtime","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-12-02T09:00:14Z","receivedAt":"2013-12-02T09:00:14Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Dec 2, 2013 at 2:40 PM, Trần Ngọc Quân <vnwildman@gmail.com> wrote:\n> On 02/12/2013 12:57, Duy Nguyen wrote:\n>>> I suggest use C preprocessor instead. The person who complete git (make debian, rpm etc. package) decide  enable it or not (disable by default). Most of people use git from distribution instead of complete it from source.\n>>>\n>>> #ifndef VSNPRINTF_OK\n>>>         setlocale(LC_CTYPE, \"C\");\n>>> #endif\n>>>\n>> A single vsnprintf is cheap enough that I would not worry about\n>> performance impact. Given a choice between this and distro\n>> maintainers, some of them do check release notes, some not so much,\n>> I'd rather go with this.\n> We can set this macro automatically  by using autoconf.\n> Add following code in configure.ac\n\nExcept that most git devs do not use autoconf. I don't know about\nother but Gentoo also packages git without autoconf. And this test\nonly means that at build time we detect this. glibc version at runtime\ncould be different, as long as they are binary compatible with the\nbuild-time one.\n\n>\n>\n> AC_LANG_CONFTEST(\n> [AC_LANG_PROGRAM([[\n> #include <stdio.h>\n> #include <locale.h>\n> #include <gnu/libc-version.h>\n>\n> #define STR \"David_K\\345gedal\"\n> ]],[[\n>     char buf[20];\n>     setlocale(LC_ALL, \"en_US.UTF-8\");\n>     if (snprintf(buf, 13, \"%.13s\", STR) < 0){\n>         printf(\"0\");\n>     }else{\n>         printf(\"1\");\n>     }\n> ]])])\n> gcc -o conftest conftest.c\n> AC_DEFINE([VSNPRINTF_OK], [m4_esyscmd([./conftest])], [Enable l10n libc\n> if vnsprintf OK])\n>\n> You can change c code here!\n>\n> --\n> Trần Ngọc Quân.\n>\n\n\n\n-- \nDuy\n"}]}