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

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

[Nguyễn Thái Ngọc Duy]

Bug 6530 [1] causes "git show v0.99.6~1" to fail with error "your
vsnprintf is broken". The workaround avoids that, but it corrupts
system error messages in non-C locales.

The bug has been fixed since 2.17. If git is built with glibc, it can
know running libc version with gnu_get_libc_version() and avoid the
workaround on fixed versions. As a side effect, the workaround is
dropped for all non-glibc systems.

Tested on Gentoo Linux, glibc 2.17, amd64.

[1] http://sourceware.org/bugzilla/show_bug.cgi?id=6530

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 We could even dlopen and look for gnu_get_libc_version at runtime and
 drop USE_GLIBC define. But there may be other problems with dl* on
 other platforms..

 Somebody should test for the other two "USE_GLIBC = YesPlease" I
 added. I don't have Debian/FreeBSD nor any non-Linux GNU systems.

 Makefile         |  5 +++++
 config.mak.uname |  3 +++
 gettext.c        | 34 ++++++++++++++++++++++++++++------
 3 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index af847f8..8df6d6d 100644
--- a/Makefile
+++ b/Makefile
@@ -66,6 +66,8 @@ all::
 # Define LIBC_CONTAINS_LIBINTL if your gettext implementation doesn't
 # need -lintl when linking.
 #
+# Define USE_GLIBC if your libc version is glibc >= 2.1.
+#
 # Define NO_MSGFMT_EXTENDED_OPTIONS if your implementation of msgfmt
 # doesn't support GNU extensions like --check and --statistics
 #
@@ -1203,6 +1205,9 @@ ifndef NO_GETTEXT
 ifndef LIBC_CONTAINS_LIBINTL
 	EXTLIBS += -lintl
 endif
+ifdef USE_GLIBC
+	BASIC_CFLAGS += -DUSE_GLIBC
+endif
 endif
 ifdef NEEDS_SOCKET
 	EXTLIBS += -lsocket
diff --git a/config.mak.uname b/config.mak.uname
index 82d549e..ffb01e0 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -33,6 +33,7 @@ ifeq ($(uname_S),Linux)
 	HAVE_PATHS_H = YesPlease
 	LIBC_CONTAINS_LIBINTL = YesPlease
 	HAVE_DEV_TTY = YesPlease
+	USE_GLIBC = YesPlease
 endif
 ifeq ($(uname_S),GNU/kFreeBSD)
 	NO_STRLCPY = YesPlease
@@ -40,6 +41,7 @@ ifeq ($(uname_S),GNU/kFreeBSD)
 	HAVE_PATHS_H = YesPlease
 	DIR_HAS_BSD_GROUP_SEMANTICS = YesPlease
 	LIBC_CONTAINS_LIBINTL = YesPlease
+	USE_GLIBC = YesPlease
 endif
 ifeq ($(uname_S),UnixWare)
 	CC = cc
@@ -236,6 +238,7 @@ ifeq ($(uname_S),GNU)
 	NO_MKSTEMPS = YesPlease
 	HAVE_PATHS_H = YesPlease
 	LIBC_CONTAINS_LIBINTL = YesPlease
+	USE_GLIBC = YesPlease
 endif
 ifeq ($(uname_S),IRIX)
 	NO_SETENV = YesPlease
diff --git a/gettext.c b/gettext.c
index 71e9545..91e679d 100644
--- a/gettext.c
+++ b/gettext.c
@@ -18,6 +18,13 @@
 #	endif
 #endif
 
+#ifdef USE_GLIBC
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE
+#endif
+#include <gnu/libc-version.h>
+#endif
+
 #ifdef GETTEXT_POISON
 int use_gettext_poison(void)
 {
@@ -30,6 +37,7 @@ int use_gettext_poison(void)
 
 #ifndef NO_GETTEXT
 static const char *charset;
+static int vsnprintf_workaround;
 static void init_gettext_charset(const char *domain)
 {
 	/*
@@ -99,9 +107,7 @@ static void init_gettext_charset(const char *domain)
 	   $ LANGUAGE= LANG=de_DE.utf8 ./test
 	   test: Kein passendes Ger?t gefunden
 
-	   In the long term we should probably see about getting that
-	   vsnprintf bug in glibc fixed, and audit our code so it won't
-	   fall apart under a non-C locale.
+	   The vsnprintf bug has been fixed since 2.17.
 
 	   Then we could simply set LC_CTYPE from the environment, which would
 	   make things like the external perror(3) messages work.
@@ -112,20 +118,36 @@ static void init_gettext_charset(const char *domain)
 	   1. http://sourceware.org/bugzilla/show_bug.cgi?id=6530
 	   2. E.g. "Content-Type: text/plain; charset=UTF-8\n" in po/is.po
 	*/
-	setlocale(LC_CTYPE, "");
+	if (vsnprintf_workaround)
+		setlocale(LC_CTYPE, "");
 	charset = locale_charset();
 	bind_textdomain_codeset(domain, charset);
-	setlocale(LC_CTYPE, "C");
+	if (vsnprintf_workaround)
+		setlocale(LC_CTYPE, "C");
 }
 
 void git_setup_gettext(void)
 {
 	const char *podir = getenv("GIT_TEXTDOMAINDIR");
 
+#ifdef USE_GLIBC
+	int major, minor;
+	const char *version = gnu_get_libc_version();
+
+	if (version && sscanf(version, "%d.%d", &major, &minor) == 2 &&
+	    (major > 2 || (major == 2 && minor >= 17)))
+		vsnprintf_workaround = 0;
+	else
+		vsnprintf_workaround = 1;
+#endif
+
 	if (!podir)
 		podir = GIT_LOCALE_PATH;
 	bindtextdomain("git", podir);
-	setlocale(LC_MESSAGES, "");
+	if (vsnprintf_workaround)
+		setlocale(LC_MESSAGES, "");
+	else
+		setlocale(LC_ALL, "");
 	init_gettext_charset("git");
 	textdomain("git");
 }
-- 
1.8.2.83.gc99314b

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

[Andreas Schwab]

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:

> diff --git a/gettext.c b/gettext.c
> index 71e9545..91e679d 100644
> --- a/gettext.c
> +++ b/gettext.c
> @@ -18,6 +18,13 @@
>  #	endif
>  #endif
>  
> +#ifdef USE_GLIBC
> +#ifndef _GNU_SOURCE
> +#define _GNU_SOURCE
> +#endif

Defining a feature test macro after any system header is included is
undefined.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

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

[Nguyễn Thái Ngọc Duy]

Bug 6530 [1] causes "git show v0.99.6~1" to fail with error "your
vsnprintf is broken". The workaround avoids that, but it corrupts
system error messages in non-C locales.

The bug has been fixed since 2.17. If git is built with glibc, it can
know running libc version with gnu_get_libc_version() and avoid the
workaround on fixed versions. The workaround is also dropped for all
non-glibc systems.

Tested on Gentoo Linux, glibc 2.17, amd64.

[1] http://sourceware.org/bugzilla/show_bug.cgi?id=6530

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 v2 removes USE_GLIBC and lets git-compat-util.h do the detection

 gettext.c         | 24 ++++++++++++++++--------
 git-compat-util.h | 11 +++++++++++
 2 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/gettext.c b/gettext.c
index 71e9545..772ab92 100644
--- a/gettext.c
+++ b/gettext.c
@@ -30,7 +30,7 @@ int use_gettext_poison(void)
 
 #ifndef NO_GETTEXT
 static const char *charset;
-static void init_gettext_charset(const char *domain)
+static void init_gettext_charset(const char *domain, int vsnprintf_broken)
 {
 	/*
 	   This trick arranges for messages to be emitted in the user's
@@ -99,9 +99,7 @@ static void init_gettext_charset(const char *domain)
 	   $ LANGUAGE= LANG=de_DE.utf8 ./test
 	   test: Kein passendes Ger?t gefunden
 
-	   In the long term we should probably see about getting that
-	   vsnprintf bug in glibc fixed, and audit our code so it won't
-	   fall apart under a non-C locale.
+	   The vsnprintf bug has been fixed since 2.17.
 
 	   Then we could simply set LC_CTYPE from the environment, which would
 	   make things like the external perror(3) messages work.
@@ -112,21 +110,31 @@ static void init_gettext_charset(const char *domain)
 	   1. http://sourceware.org/bugzilla/show_bug.cgi?id=6530
 	   2. E.g. "Content-Type: text/plain; charset=UTF-8\n" in po/is.po
 	*/
-	setlocale(LC_CTYPE, "");
+	if (vsnprintf_broken)
+		setlocale(LC_CTYPE, "");
 	charset = locale_charset();
 	bind_textdomain_codeset(domain, charset);
-	setlocale(LC_CTYPE, "C");
+	if (vsnprintf_broken)
+		setlocale(LC_CTYPE, "C");
 }
 
 void git_setup_gettext(void)
 {
 	const char *podir = getenv("GIT_TEXTDOMAINDIR");
+	const char *version = glibc_version();
+	int major, minor, vsnprintf_broken;
+
+	if (version && sscanf(version, "%d.%d", &major, &minor) == 2 &&
+	    (major > 2 || (major == 2 && minor >= 17)))
+		vsnprintf_broken = 0;
+	else
+		vsnprintf_broken = 1;
 
 	if (!podir)
 		podir = GIT_LOCALE_PATH;
 	bindtextdomain("git", podir);
-	setlocale(LC_MESSAGES, "");
-	init_gettext_charset("git");
+	setlocale(vsnprintf_broken ? LC_MESSAGES : LC_ALL, "");
+	init_gettext_charset("git", vsnprintf_broken);
 	textdomain("git");
 }
 
diff --git a/git-compat-util.h b/git-compat-util.h
index 7776f12..967f452 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -488,11 +488,22 @@ extern int git_vsnprintf(char *str, size_t maxsize,
 
 #ifdef __GLIBC_PREREQ
 #if __GLIBC_PREREQ(2, 1)
+#include <gnu/libc-version.h>
+#define glibc_version() gnu_get_libc_version()
 #define HAVE_STRCHRNUL
 #define HAVE_MEMPCPY
 #endif
 #endif
 
+#ifndef glibc_version
+#ifdef __GNU_LIBRARY__
+#define glibc_version() NULL
+#else
+/* non-glibc platforms, see git_setup_gettext() for "2.17" */
+#define glibc_version() "2.17"
+#endif
+#endif
+
 #ifndef HAVE_STRCHRNUL
 #define strchrnul gitstrchrnul
 static inline char *gitstrchrnul(const char *s, int c)
-- 
1.8.2.83.gc99314b

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

[Torsten Bögershausen]

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 ?)
[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.
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 ?
------------

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

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

[Torsten Bögershausen]

> gnu_glibc_version()=2.11.3 /* Ubuntu 10.4, no updates */
Sorry, Copy-paste error:
2.11.1

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

[Duy Nguyen]

On Sun, Dec 1, 2013 at 6:01 AM, Torsten Bögershausen <tboegi@web.de> wrote:
> 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.

> [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, "");

> 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 :(

> ------------
>
> 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

[PATCH v3] gettext.c: detect the vsnprintf bug at runtime

[Nguyễn Thái Ngọc Duy]

Bug 6530 [1] in glibc causes "git show v0.99.6~1" to fail with error
"your vsnprintf is broken". The workaround avoids that, but it
corrupts system error messages in non-C locales.

The bug has been fixed since 2.17. We could know running glibc version
with gnu_get_libc_version(). But version is not a sure way to detect
the bug because downstream may back port the fix to older versions. Do
a runtime test that immitates the call flow that leads to "your
vsnprintf is broken". Only enable the workaround if the test fails.

Tested on Gentoo Linux, glibc 2.16.0 and 2.17, amd64.

[1] http://sourceware.org/bugzilla/show_bug.cgi?id=6530

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 v3 goes with runtime test instead of version check.

 gettext.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/gettext.c b/gettext.c
index 71e9545..eed7c7f 100644
--- a/gettext.c
+++ b/gettext.c
@@ -29,6 +29,17 @@ int use_gettext_poison(void)
 #endif
 
 #ifndef NO_GETTEXT
+static int test_vsnprintf(const char *fmt, ...)
+{
+	char buf[26];
+	int ret;
+	va_list ap;
+	va_start(ap, fmt);
+	ret = vsnprintf(buf, sizeof(buf), fmt, ap);
+	va_end(ap);
+	return ret;
+}
+
 static const char *charset;
 static void init_gettext_charset(const char *domain)
 {
@@ -99,9 +110,7 @@ static void init_gettext_charset(const char *domain)
 	   $ LANGUAGE= LANG=de_DE.utf8 ./test
 	   test: Kein passendes Ger?t gefunden
 
-	   In the long term we should probably see about getting that
-	   vsnprintf bug in glibc fixed, and audit our code so it won't
-	   fall apart under a non-C locale.
+	   The vsnprintf bug has been fixed since glibc 2.17.
 
 	   Then we could simply set LC_CTYPE from the environment, which would
 	   make things like the external perror(3) messages work.
@@ -115,7 +124,9 @@ static void init_gettext_charset(const char *domain)
 	setlocale(LC_CTYPE, "");
 	charset = locale_charset();
 	bind_textdomain_codeset(domain, charset);
-	setlocale(LC_CTYPE, "C");
+	/* the string is taken from v0.99.6~1 */
+	if (test_vsnprintf("%.*s", 13, "David_K\345gedal") < 0)
+		setlocale(LC_CTYPE, "C");
 }
 
 void git_setup_gettext(void)
-- 
1.8.2.83.gc99314b

Re: [PATCH v3] gettext.c: detect the vsnprintf bug at runtime

[Trần Ngọc Quân]

On 01/12/2013 09:45, Nguyễn Thái Ngọc Duy wrote:
> Bug 6530 [1] in glibc causes "git show v0.99.6~1" to fail with error
> "your vsnprintf is broken". The workaround avoids that, but it
> corrupts system error messages in non-C locales.
>
> The bug has been fixed since 2.17. We could know running glibc version
> with gnu_get_libc_version(). But version is not a sure way to detect
> the bug because downstream may back port the fix to older versions. Do
> a runtime test that immitates the call flow that leads to "your
> vsnprintf is broken". Only enable the workaround if the test fails.
>
> Tested on Gentoo Linux, glibc 2.16.0 and 2.17, amd64.
>
> [1] http://sourceware.org/bugzilla/show_bug.cgi?id=6530
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  v3 goes with runtime test instead of version check.
>
>  gettext.c | 19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/gettext.c b/gettext.c
> index 71e9545..eed7c7f 100644
> --- a/gettext.c
> +++ b/gettext.c
> @@ -29,6 +29,17 @@ int use_gettext_poison(void)
>  #endif
>  
>  #ifndef NO_GETTEXT
> +static int test_vsnprintf(const char *fmt, ...)
> +{
> +	char buf[26];
> +	int ret;
> +	va_list ap;
> +	va_start(ap, fmt);
> +	ret = vsnprintf(buf, sizeof(buf), fmt, ap);
> +	va_end(ap);
> +	return ret;
> +}
> +
this function alway run each time we run git commad while libc is
static. It is waste.
> +	/* the string is taken from v0.99.6~1 */
> +	if (test_vsnprintf("%.*s", 13, "David_K\345gedal") < 0)
> +		setlocale(LC_CTYPE, "C");
>  }
>  
>  void git_setup_gettext(void)

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.

#ifndef VSNPRINTF_OK
	setlocale(LC_CTYPE, "C");
#endif


-- 
Trần Ngọc Quân.

Re: [PATCH v3] gettext.c: detect the vsnprintf bug at runtime

[Duy Nguyen]

On Mon, Dec 2, 2013 at 7:31 AM, Trần Ngọc Quân <vnwildman@gmail.com> wrote:
>> --- a/gettext.c
>> +++ b/gettext.c
>> @@ -29,6 +29,17 @@ int use_gettext_poison(void)
>>  #endif
>>
>>  #ifndef NO_GETTEXT
>> +static int test_vsnprintf(const char *fmt, ...)
>> +{
>> +     char buf[26];
>> +     int ret;
>> +     va_list ap;
>> +     va_start(ap, fmt);
>> +     ret = vsnprintf(buf, sizeof(buf), fmt, ap);
>> +     va_end(ap);
>> +     return ret;
>> +}
>> +
> this function alway run each time we run git commad while libc is
> static. It is waste.
>> +     /* the string is taken from v0.99.6~1 */
>> +     if (test_vsnprintf("%.*s", 13, "David_K\345gedal") < 0)
>> +             setlocale(LC_CTYPE, "C");
>>  }
>>
>>  void git_setup_gettext(void)
>
> 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.
>
> #ifndef VSNPRINTF_OK
>         setlocale(LC_CTYPE, "C");
> #endif
>

A single vsnprintf is cheap enough that I would not worry about
performance impact. Given a choice between this and distro
maintainers, some of them do check release notes, some not so much,
I'd rather go with this.
-- 
Duy

Re: [PATCH v3] gettext.c: detect the vsnprintf bug at runtime

[Trần Ngọc Quân]

On 02/12/2013 12:57, Duy Nguyen wrote:
>> 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.
>>
>> #ifndef VSNPRINTF_OK
>>         setlocale(LC_CTYPE, "C");
>> #endif
>>
> A single vsnprintf is cheap enough that I would not worry about
> performance impact. Given a choice between this and distro
> maintainers, some of them do check release notes, some not so much,
> I'd rather go with this.
We can set this macro automatically  by using autoconf.
Add following code in configure.ac


AC_LANG_CONFTEST(
[AC_LANG_PROGRAM([[
#include <stdio.h>
#include <locale.h>
#include <gnu/libc-version.h>

#define STR "David_K\345gedal"
]],[[
    char buf[20];
    setlocale(LC_ALL, "en_US.UTF-8");
    if (snprintf(buf, 13, "%.13s", STR) < 0){
        printf("0");
    }else{
        printf("1");
    }
]])])
gcc -o conftest conftest.c
AC_DEFINE([VSNPRINTF_OK], [m4_esyscmd([./conftest])], [Enable l10n libc
if vnsprintf OK])

You can change c code here!

-- 
Trần Ngọc Quân.

Re: [PATCH v3] gettext.c: detect the vsnprintf bug at runtime

[Trần Ngọc Quân]

On 02/12/2013 14:40, Trần Ngọc Quân wrote:
> On 02/12/2013 12:57, Duy Nguyen wrote:
>>> 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.
>>>
>>> #ifndef VSNPRINTF_OK
>>>         setlocale(LC_CTYPE, "C");
>>> #endif
>>>
>> A single vsnprintf is cheap enough that I would not worry about
>> performance impact. Given a choice between this and distro
>> maintainers, some of them do check release notes, some not so much,
>> I'd rather go with this.
> We can set this macro automatically  by using autoconf.
> Add following code in configure.ac
>
>
> AC_LANG_CONFTEST(
> [AC_LANG_PROGRAM([[
> #include <stdio.h>
> #include <locale.h>
> #include <gnu/libc-version.h>
>
> #define STR "David_K\345gedal"
> ]],[[
>     char buf[20];
>     setlocale(LC_ALL, "en_US.UTF-8");
>     if (snprintf(buf, 13, "%.13s", STR) < 0){
>         printf("0");
>     }else{
>         printf("1");
>     }
> ]])])
> gcc -o conftest conftest.c
> AC_DEFINE([VSNPRINTF_OK], [m4_esyscmd([./conftest])], [Enable l10n libc
> if vnsprintf OK])
>
> You can change c code here!
>
Sorry I'm wrong,  m4_esyscmd don't work as I spect
Change to:
AC_LANG_CONFTEST(
[AC_LANG_PROGRAM([[
#include <stdio.h>
#include <locale.h>
#include <gnu/libc-version.h>

#define STR "David_K\345gedal"
]],[[
    char buf[20];
    setlocale(LC_CTYPE, "en_US.UTF-8");
    if (snprintf(buf, 13, "%.13s", STR) < 0){
        printf("0");
    }else{
        printf("1");
    }
]])])
gcc -o conftest conftest.c
VSNPRINTF=$(./conftest)
AC_DEFINE_UNQUOTED([VSNPRINTF_OK], [ $VSNPRINTF ], [Enable libc if
vnsprintf OK])

don't missing run this:
$ autoconf && autoheader

My os (ubuntu 12.04) don't pass this test (libc6:i386     2.15-0ubuntu)

$ grep VSNPRINTF_OK  config.h
#define VSNPRINTF_OK 0

-- 
Trần Ngọc Quân.

Re: [PATCH v3] gettext.c: detect the vsnprintf bug at runtime

[Duy Nguyen]

On Mon, Dec 2, 2013 at 2:40 PM, Trần Ngọc Quân <vnwildman@gmail.com> wrote:
> On 02/12/2013 12:57, Duy Nguyen wrote:
>>> 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.
>>>
>>> #ifndef VSNPRINTF_OK
>>>         setlocale(LC_CTYPE, "C");
>>> #endif
>>>
>> A single vsnprintf is cheap enough that I would not worry about
>> performance impact. Given a choice between this and distro
>> maintainers, some of them do check release notes, some not so much,
>> I'd rather go with this.
> We can set this macro automatically  by using autoconf.
> Add following code in configure.ac

Except that most git devs do not use autoconf. I don't know about
other but Gentoo also packages git without autoconf. And this test
only means that at build time we detect this. glibc version at runtime
could be different, as long as they are binary compatible with the
build-time one.

>
>
> AC_LANG_CONFTEST(
> [AC_LANG_PROGRAM([[
> #include <stdio.h>
> #include <locale.h>
> #include <gnu/libc-version.h>
>
> #define STR "David_K\345gedal"
> ]],[[
>     char buf[20];
>     setlocale(LC_ALL, "en_US.UTF-8");
>     if (snprintf(buf, 13, "%.13s", STR) < 0){
>         printf("0");
>     }else{
>         printf("1");
>     }
> ]])])
> gcc -o conftest conftest.c
> AC_DEFINE([VSNPRINTF_OK], [m4_esyscmd([./conftest])], [Enable l10n libc
> if vnsprintf OK])
>
> You can change c code here!
>
> --
> Trần Ngọc Quân.
>



-- 
Duy