{"thread":{"id":"12493","subject":"[PATCH] Add compat/vsnprintf.c for systems that returns -1 on maxsize reached","startedAt":"2008-03-04T13:59:28Z","lastAt":"2008-03-10T10:05:14Z","messageCount":26,"participants":["Michal Rokos","Johannes Schindelin","Morten Welinder","Finn Arne Gangstad","Johannes Sixt","Wayne Davison","Jeff King","Mike Ralphson","Robert Haines","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"70941","messageId":"200803041459.29000.michal.rokos@nextsoft.cz","threadId":"12493","inReplyTo":null,"subject":"[PATCH] Add compat/vsnprintf.c for systems that returns -1 on maxsize reached","fromName":"Michal Rokos","fromEmail":"michal.rokos@nextsoft.cz","sentAt":"2008-03-04T13:59:28Z","receivedAt":"2008-03-04T13:59:28Z","isPatch":true,"sender":{"key":"michal.rokos@nextsoft.cz","avatar":null},"body":"This PATCH is NOT intended to be merged (yet).\n\nSome systems (namely HPUX) return -1 when maxsize in vsnprintf() is reached. \nSo replace that broken vsnprintf() with our own that returns correct value \nupon overflow.\n\nCould anybody give it some testing since I don't know how many broken systems \nare out there?\n\nIf anybody could think of some better define than BROKEN_VSNPRINTF, I'm all \nears.\n\nLinux is OK, HPUX is detected to be broken. On HPUX (11.23) test suite with \ndefined BROKEN_VSNPRINTF fails in 8 *.sh testsuites, without it it fails in \n140 *.sh testsuites (out of 241).\n\nSigned-off-by: Michal Rokos \n\ndiff --git a/Makefile b/Makefile\nindex ca5aad9..a1dbf1d 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3,6 +3,10 @@ all::\n \n # Define V=1 to have a more verbose compile.\n #\n+# Define BROKEN_VSNPRINTF if your are on a system which vsnprintf() return\n+# something else (typically -1) instead of number of characters which would\n+# have been written to the final string if enough space had been available.\n+#\n # Define FREAD_READS_DIRECTORIES if your are on a system which succeeds\n # when attempting to read from an fopen'ed directory.\n #\n@@ -526,6 +530,7 @@ ifeq ($(uname_S),HP-UX)\n \tNO_UNSETENV = YesPlease\n \tNO_HSTRERROR = YesPlease\n \tNO_SYS_SELECT_H = YesPlease\n+\tBROKEN_VSNPRINTF = UnfortunatelyYes\n endif\n ifneq (,$(findstring arm,$(uname_M)))\n \tARM_SHA1 = YesPlease\n@@ -629,6 +634,10 @@ endif\n ifdef NO_C99_FORMAT\n \tBASIC_CFLAGS += -DNO_C99_FORMAT\n endif\n+ifdef BROKEN_VSNPRINTF\n+\tCOMPAT_CFLAGS += -DBROKEN_VSNPRINTF\n+\tCOMPAT_OBJS += compat/vsnprintf.o\n+endif\n ifdef FREAD_READS_DIRECTORIES\n \tCOMPAT_CFLAGS += -DFREAD_READS_DIRECTORIES\n \tCOMPAT_OBJS += compat/fopen.o\ndiff --git a/config.mak.in b/config.mak.in\nindex ee6c33d..aaa98c6 100644\n--- a/config.mak.in\n+++ b/config.mak.in\n@@ -46,3 +46,4 @@ NO_MKDTEMP=@NO_MKDTEMP@\n NO_ICONV=@NO_ICONV@\n OLD_ICONV=@OLD_ICONV@\n NO_DEFLATE_BOUND=@NO_DEFLATE_BOUND@\n+BROKEN_VSNPRINTF=@BROKEN_VSNPRINTF@\ndiff --git a/configure.ac b/configure.ac\nindex 85d7ef5..d4bb2b3 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -326,6 +326,37 @@ else\n \tNO_C99_FORMAT=\n fi\n AC_SUBST(NO_C99_FORMAT)\n+#\n+# Define BROKEN_VSNPRINTF if your are on a system which vsnprintf() return\n+# something else (typically -1) instead of number of characters which would\n+# have been written to the final string if enough space had been available.\n+AC_CACHE_CHECK([whether vsnprintf() is broken],\n+ [ac_cv_broken_vsnprintf],\n+[\n+AC_RUN_IFELSE(\n+\t[AC_LANG_PROGRAM([AC_INCLUDES_DEFAULT\n+\t\t#include \"stdarg.h\"\n+\n+\t\tint test_vsnprintf(char *str, size_t maxsize, const char *format, ...)\n+\t\t{\n+\t\t int ret;\n+\t\t va_list ap;\n+\t\t va_start(ap, format);\n+\t\t ret = vsnprintf(str, maxsize, format, ap);\n+\t\t va_end(ap);\n+ return ret;\n+\t\t}],\n+\t\t[[char buf[1];\n+\t\t if (test_vsnprintf(buf, 1, \"%s\", \"12345\") != 5) return 1]])],\n+\t[ac_cv_broken_vsnprintf=no],\n+\t[ac_cv_broken_vsnprintf=yes])\n+])\n+if test $ac_cv_broken_vsnprintf = yes; then\n+\tBROKEN_VSNPRINTF=UnfortunatelyYes\n+else\n+\tBROKEN_VSNPRINTF=\n+fi\n+AC_SUBST(BROKEN_VSNPRINTF)\n \n \n ## Checks for library functions.\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 2a40703..5c392f8 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -209,6 +209,12 @@ void *gitmemmem(const void *haystack, size_t haystacklen,\n extern FILE *git_fopen(const char*, const char*);\n #endif\n \n+#ifdef BROKEN_VSNPRINTF\n+#define vsnprintf git_vsnprintf\n+extern int git_vsnprintf(char *str, size_t maxsize,\n+ const char *format, va_list ap);\n+#endif\n+\n #ifdef __GLIBC_PREREQ\n #if __GLIBC_PREREQ(2, 1)\n #define HAVE_STRCHRNUL\ndiff --git a/dev/null b/compat/vsnprintf.c\nnew file mode 100644\nindex 0000000..263e00e\n--- /dev/null\n+++ b/compat/vsnprintf.c\n@@ -0,0 +1,20 @@\n+#include \"../git-compat-util.h\"\n+\n+#undef vsnprintf\n+int git_vsnprintf(char *str, size_t maxsize, const char *format, va_list ap);\n+{\n+\tint ret = vsnprintf(s, maxsize, format, ap);\n+\tif (ret != -1 ) return ret;\n+\n+\ts = NULL;\n+\twhile ( ret == -1 )\n+\t{\n+\t\tmaxsize = (maxsize*3)/2;\n+\t\ts = realloc(s, maxsize);\n+\t\tif (! s) return -1;\n+\t\tret = vsnprintf(s, maxsize, format, ap);\n+\t}\n+\tfree(s);\n+\treturn ret;\n+}\n+\n"},{"id":"70942","messageId":"alpine.LSU.1.00.0803041409070.22527@racer.site","threadId":"12493","inReplyTo":"200803041459.29000.michal.rokos@nextsoft.cz","subject":"Re: [PATCH] Add compat/vsnprintf.c for systems that returns -1 on maxsize reached","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-03-04T14:09:19Z","receivedAt":"2008-03-04T14:09:19Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 4 Mar 2008, Michal Rokos wrote:\n\n> This PATCH is NOT intended to be merged (yet).\n\nSee also\n\nhttp://article.gmane.org/gmane.comp.version-control.git/75280/match=vsnprintf\n\nCiao,\nDscho\n"},{"id":"70947","messageId":"20080304140930.GA23335@pvv.org","threadId":"12493","inReplyTo":"200803041459.29000.michal.rokos@nextsoft.cz","subject":"Re: [PATCH] Add compat/vsnprintf.c for systems that returns -1 on maxsize reached","fromName":"Finn Arne Gangstad","fromEmail":"finnag@pvv.org","sentAt":"2008-03-04T14:09:31Z","receivedAt":"2008-03-04T14:09:31Z","isPatch":true,"sender":{"key":"finnag@pvv.org","avatar":"https://gravatar.com/avatar/b421ddd58c3f0f93aa473e17b98bb8d53c221fef741746bc8cb59fae4ec6d95e?d=mp&s=160"},"body":"On Tue, Mar 04, 2008 at 02:59:28PM +0100, Michal Rokos wrote:\n\n> +\twhile ( ret == -1 )\n> +\t{\n> +\t\tmaxsize = (maxsize*3)/2;\n> +\t\ts = realloc(s, maxsize);\n> +\t\tif (! s) return -1;\n> +\t\tret = vsnprintf(s, maxsize, format, ap); /* <--- UNSAFE! */\n> +\t}\n\nThis is not generally safe, you cannot call vsnprintf multiple times\nwith the same ap on all architectures. You need va_copy (or __va_copy,\nor VA_COPY, differs a bit between different architectures, especially\none the ones with a broken vsnprintf I guess..)\n\n- Finn Arne\n"},{"id":"70943","messageId":"118833cc0803040612g38dfc9f8tdb33ef866f6752e2@mail.gmail.com","threadId":"12493","inReplyTo":"200803041459.29000.michal.rokos@nextsoft.cz","subject":"Re: [PATCH] Add compat/vsnprintf.c for systems that returns -1 on maxsize reached","fromName":"Morten Welinder","fromEmail":"mwelinder@gmail.com","sentAt":"2008-03-04T14:12:18Z","receivedAt":"2008-03-04T14:12:18Z","isPatch":true,"sender":{"key":"mwelinder@gmail.com","avatar":null},"body":"That leaks in the out-of-memory case. Not terribly important, but still.\n\nMorten\n"},{"id":"70972","messageId":"47CD7686.9040501@viscovery.net","threadId":"12493","inReplyTo":"20080304140930.GA23335@pvv.org","subject":"Re: [PATCH] Add compat/vsnprintf.c for systems that returns -1 on maxsize reached","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-03-04T16:19:18Z","receivedAt":"2008-03-04T16:19:18Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Finn Arne Gangstad schrieb:\n> On Tue, Mar 04, 2008 at 02:59:28PM +0100, Michal Rokos wrote:\n> \n>> +\twhile ( ret == -1 )\n>> +\t{\n>> +\t\tmaxsize = (maxsize*3)/2;\n>> +\t\ts = realloc(s, maxsize);\n>> +\t\tif (! s) return -1;\n>> +\t\tret = vsnprintf(s, maxsize, format, ap); /* <--- UNSAFE! */\n>> +\t}\n> \n> This is not generally safe, you cannot call vsnprintf multiple times\n> with the same ap on all architectures. You need va_copy (or __va_copy,\n> or VA_COPY, differs a bit between different architectures, especially\n> one the ones with a broken vsnprintf I guess..)\n\nTrue. But...\n\nThis replacement of vsnprintf will not be needed on all architectures, but\nonly on some. And on these we can test in advance whether we can get away\nwithout va_copy (et.al.). A note next to the configuration setting about\nthis would be in order, I think.\n\nFurthermore, on systems where vsnprintf is broken in this way, va_copy is\nlikely not available.\n\n-- Hannes\n\n"},{"id":"70975","messageId":"47CD78C9.80003@viscovery.net","threadId":"12493","inReplyTo":"200803041459.29000.michal.rokos@nextsoft.cz","subject":"Re: [PATCH] Add compat/vsnprintf.c for systems that returns -1 on maxsize reached","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-03-04T16:28:57Z","receivedAt":"2008-03-04T16:28:57Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Michal Rokos schrieb:\n> If anybody could think of some better define than BROKEN_VSNPRINTF, I'm all \n> ears.\n\nCLUELESS_OVERFLOWN_VSNPRINTF?\n\nBecause we have the same issue on Windows where vsnprintf returns -1 on\noverflow.\n\nBut there is also another complication: The size parameter of the system's\nvsnprintf must not count the trailing NUL, i.e. the buffer must actually\nhave space for one extra byte, whereas the POSIX version must count NUL.\nCan you check whether your vsnprintf has this flaw, too? If it doesn't, we\nwould need another configuration variable so that we can distinguish these\ntwo kinds of brokenness.\n\n-- Hannes\n"},{"id":"71014","messageId":"20080304235131.GA27590@herod.dreamhost.com","threadId":"12493","inReplyTo":"47CD78C9.80003@viscovery.net","subject":"Re: [PATCH] Add compat/vsnprintf.c for systems that returns -1 on maxsize reached","fromName":"Wayne Davison","fromEmail":"wayne@opencoder.net","sentAt":"2008-03-04T23:51:31Z","receivedAt":"2008-03-04T23:51:31Z","isPatch":true,"sender":{"key":"wayne@opencoder.net","avatar":"https://gravatar.com/avatar/d55d81825271b1bfe65e57e4e04297d4119aa03c6a1c71d2ff2812b9b4be9f45?d=mp&s=160"},"body":"On Tue, Mar 04, 2008 at 05:28:57PM +0100, Johannes Sixt wrote:\n> But there is also another complication: The size parameter of the system's\n> vsnprintf must not count the trailing NUL, i.e. the buffer must actually\n> have space for one extra byte, whereas the POSIX version must count NUL.\n\nFYI, rsync's configure script has a check for both those problems. It\nsets HAVE_C99_VSNPRINTF if vsnprintf() works right. If that fails,\nrsync uses its own lib/snprintf.c implementation (that I believe comes\nfrom Samba). That file could be included in git, if desired.\n\n..wayne..\n"},{"id":"71049","messageId":"200803050937.40345.michal.rokos@nextsoft.cz","threadId":"12493","inReplyTo":"47CD78C9.80003@viscovery.net","subject":"Re: [PATCH] Add compat/vsnprintf.c for systems that returns -1 on maxsize reached","fromName":"Michal Rokos","fromEmail":"michal.rokos@nextsoft.cz","sentAt":"2008-03-05T08:37:39Z","receivedAt":"2008-03-05T08:37:39Z","isPatch":true,"sender":{"key":"michal.rokos@nextsoft.cz","avatar":null},"body":"Hello,\n\nOn Tuesday 04 March 2008 17:28:57 Johannes Sixt wrote:\n> Michal Rokos schrieb:\n> > If anybody could think of some better define than BROKEN_VSNPRINTF, I'm\n> > all ears.\n>\n> CLUELESS_OVERFLOWN_VSNPRINTF?\n\nHeh. My first one was VSNPRINTF_RETURNS_MINUS_1...\n\n> Because we have the same issue on Windows where vsnprintf returns -1 on\n> overflow.\n>\n> But there is also another complication: The size parameter of the system's\n> vsnprintf must not count the trailing NUL, i.e. the buffer must actually\n> have space for one extra byte, whereas the POSIX version must count NUL.\n> Can you check whether your vsnprintf has this flaw, too? If it doesn't, we\n> would need another configuration variable so that we can distinguish these\n> two kinds of brokenness.\n\nI did my homework and prepared this:\n\n#include \"stdio.h\"\n#include \"stdarg.h\"\n\nint test_vsnprintf(char *str, size_t maxsize, const char *format, ...)\n{\n int ret;\n va_list ap;\n\n va_start(ap, format);\n ret = vsnprintf(str, maxsize, format, ap);\n va_end(ap);\n\n return ret;\n}\n\nint main(void)\n{\n char buf[10];\n int ret;\n\n ret = test_vsnprintf(buf, 1, \"%s\", \"12345\");\n printf(\"case1: %d\\n\", ret);\n\n ret = test_vsnprintf(buf, 5, \"%s\", \"12345\");\n printf(\"case2: %d\\n\", ret);\n\n ret = test_vsnprintf(buf, 6, \"%s\", \"12345\");\n printf(\"case3: %d\\n\", ret);\n\n ret = test_vsnprintf(buf, 10, \"%s\", \"12345\");\n printf(\"case4: %d\\n\", ret);\n\n return 0;\n}\n\nwhich returns:\nLinux 2.6.25-rc3-mr i686\ncase1: 5\ncase2: 5\ncase3: 5\ncase4: 5\n\nHP-UX B.11.11 9000/800\ncase1: -1\ncase2: -1\ncase3: 5\ncase4: 5\n\nHP-UX B.11.23 ia64\ncase1: -1\ncase2: -1\ncase3: 5\ncase4: 5\n\nSo HPUX impl is the same as a Windows one. So we can share the replacement.\nPlease note that there's no va_copy() on HPUX.\n\nI think that Tru64 has the same issue, but I have no HW to test it on now.\n\nCould somebody else try to run testcase above on some other OSes?\n\nThanks\n\nMichal\n"},{"id":"71051","messageId":"20080305084427.GA18285@sigill.intra.peff.net","threadId":"12493","inReplyTo":"200803050937.40345.michal.rokos@nextsoft.cz","subject":"Re: [PATCH] Add compat/vsnprintf.c for systems that returns -1 on maxsize reached","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-03-05T08:44:28Z","receivedAt":"2008-03-05T08:44:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 05, 2008 at 09:37:39AM +0100, Michal Rokos wrote:\n\n> Could somebody else try to run testcase above on some other OSes?\n\nSunOS 5.8 correctly returns '5' in each case.\n\n-Peff\n"},{"id":"71057","messageId":"47CE6552.60308@viscovery.net","threadId":"12493","inReplyTo":"200803050937.40345.michal.rokos@nextsoft.cz","subject":"Re: [PATCH] Add compat/vsnprintf.c for systems that returns -1 on maxsize reached","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-03-05T09:18:10Z","receivedAt":"2008-03-05T09:18:10Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Michal Rokos schrieb:\n> HP-UX B.11.11 9000/800\n> case1: -1\n> case2: -1\n> case3: 5\n> case4: 5\n> \n> HP-UX B.11.23 ia64\n> case1: -1\n> case2: -1\n> case3: 5\n> case4: 5\n> \n> So HPUX impl is the same as a Windows one. So we can share the replacement.\n> Please note that there's no va_copy() on HPUX.\n\nIt's not the same on Windows, which returns:\n\ncase1: -1\ncase2: 5\ncase3: 5\ncase4: 5\n\nIOW, HPUX et.al. take the *size* of the buffer, whereas Windows takes the\nmaximal number of characters to write excluding NUL.\n\nWhich means that we need another configuration variable on Windows:\n\n -DSNPRINTF_SIZE_CORR=1\n\nand\n\n#ifndef SNPRINTF_SIZE_CORR\n#define SNPRINTF_SIZE_CORR 0\n#endif\n\nin the replacement implementation. But I can do this myself in the course\nof the MinGW port.\n\nBTW, this is not only an issue of vsnprintf, but also of snprintf!\n\n-- Hannes\n"},{"id":"71058","messageId":"e2b179460803050122g4648270eqd65549f4db39b14b@mail.gmail.com","threadId":"12493","inReplyTo":"200803050937.40345.michal.rokos@nextsoft.cz","subject":"Re: [PATCH] Add compat/vsnprintf.c for systems that returns -1 on maxsize reached","fromName":"Mike Ralphson","fromEmail":"mike.ralphson@gmail.com","sentAt":"2008-03-05T09:22:25Z","receivedAt":"2008-03-05T09:22:25Z","isPatch":true,"sender":{"key":"mike.ralphson@gmail.com","avatar":"https://avatars.githubusercontent.com/u/21603?v=4"},"body":"On 05/03/2008, Michal Rokos wrote:\n> Could somebody else try to run testcase above on some other OSes?\n\nAIX isis 3 5 00C0FEDC4C00\ncase1: 5\ncase2: 5\ncase3: 5\ncase4: 5\n\n\n"},{"id":"71065","messageId":"F5DC9F11-FD88-4713-AD1A-6566C345852A@manchester.ac.uk","threadId":"12493","inReplyTo":"200803050937.40345.michal.rokos@nextsoft.cz","subject":"Re: [PATCH] Add compat/vsnprintf.c for systems that returns -1 on maxsize reached","fromName":"Robert Haines","fromEmail":"rhaines@manchester.ac.uk","sentAt":"2008-03-05T10:35:25Z","receivedAt":"2008-03-05T10:35:25Z","isPatch":true,"sender":{"key":"rhaines@manchester.ac.uk","avatar":null},"body":"\n> Could somebody else try to run testcase above on some other OSes?\n\n\nMac OS X Darwin 8.11.1 i386 (Tiger)\n\ncase1: 5\ncase2: 5\ncase3: 5\ncase4: 5\n\nCheers,\nRob\n\n"},{"id":"71071","messageId":"200803051455.57148.michal.rokos@nextsoft.cz","threadId":"12493","inReplyTo":"47CE6552.60308@viscovery.net","subject":"Re: [PATCH] Add compat/vsnprintf.c for systems that returns -1 on maxsize reached","fromName":"Michal Rokos","fromEmail":"michal.rokos@nextsoft.cz","sentAt":"2008-03-05T13:55:57Z","receivedAt":"2008-03-05T13:55:57Z","isPatch":true,"sender":{"key":"michal.rokos@nextsoft.cz","avatar":null},"body":"Hello,\n\nOn Wednesday 05 March 2008 10:18:10 Johannes Sixt wrote:\n> It's not the same on Windows, which returns:\n> case1: -1\n> case2: 5\n> case3: 5\n> case4: 5\n>\n> BTW, this is not only an issue of vsnprintf, but also of snprintf!\n\nHmm, HPUX has the same issue for snprint() as is for vsnprintf().\n\nDo you think that following patch suffices your needs. Please note that it \nactually copies data to str.\n\nSigned-off-by: Michal Rokos \n\ndiff --git a/Makefile b/Makefile\nindex ca5aad9..49d5ab6 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3,6 +3,14 @@ all::\n \n # Define V=1 to have a more verbose compile.\n #\n+# Define SNPRINTF_RETURNS_BOGUS if your are on a system which snprintf()\n+# returns -1 instead of number of characters which would have been written\n+# to the final string if enough space had been available.\n+#\n+# Define VSNPRINTF_RETURNS_BOGUS if your are on a system which vsnprintf()\n+# returns -1 instead of number of characters which would have been written\n+# to the final string if enough space had been available.\n+#\n # Define FREAD_READS_DIRECTORIES if your are on a system which succeeds\n # when attempting to read from an fopen'ed directory.\n #\n@@ -629,6 +637,14 @@ endif\n ifdef NO_C99_FORMAT\n \tBASIC_CFLAGS += -DNO_C99_FORMAT\n endif\n+ifdef SNPRINTF_RETURNS_BOGUS\n+\tCOMPAT_CFLAGS += -DSNPRINTF_RETURNS_BOGUS\n+\tCOMPAT_OBJS += compat/snprintf.o\n+endif\n+ifdef VSNPRINTF_RETURNS_BOGUS\n+\tCOMPAT_CFLAGS += -DVSNPRINTF_RETURNS_BOGUS\n+\tCOMPAT_OBJS += compat/snprintf.o\n+endif\n ifdef FREAD_READS_DIRECTORIES\n \tCOMPAT_CFLAGS += -DFREAD_READS_DIRECTORIES\n \tCOMPAT_OBJS += compat/fopen.o\ndiff --git a/config.mak.in b/config.mak.in\nindex ee6c33d..a10a4af 100644\n--- a/config.mak.in\n+++ b/config.mak.in\n@@ -46,3 +46,5 @@ NO_MKDTEMP=@NO_MKDTEMP@\n NO_ICONV=@NO_ICONV@\n OLD_ICONV=@OLD_ICONV@\n NO_DEFLATE_BOUND=@NO_DEFLATE_BOUND@\n+SNPRINTF_RETURNS_BOGUS=@SNPRINTF_RETURNS_BOGUS@\n+VSNPRINTF_RETURNS_BOGUS=@VSNPRINTF_RETURNS_BOGUS@\ndiff --git a/configure.ac b/configure.ac\nindex 85d7ef5..b902888 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -326,6 +326,57 @@ else\n \tNO_C99_FORMAT=\n fi\n AC_SUBST(NO_C99_FORMAT)\n+#\n+# Define SNPRINTF_RETURNS_BOGUS if your are on a system which snprintf()\n+# returns -1 instead of number of characters which would have been written\n+# to the final string if enough space had been available.\n+AC_CACHE_CHECK([whether snprintf() returns bogus],\n+ [ac_cv_snprintf_returns_bogus],\n+[\n+AC_RUN_IFELSE(\n+\t[AC_LANG_PROGRAM([AC_INCLUDES_DEFAULT],\n+\t\t[[char buf[1];\n+\t\t if (snprintf(bug, 1, \"%s\", \"12345\") != 5) return 1]])],\n+\t[ac_cv_snprintf_returns_bogus=no],\n+\t[ac_cv_snprintf_returns_bogus=yes])\n+])\n+if test $ac_cv_snprintf_returns_bogus = yes; then\n+\tSNPRINTF_RETURNS_BOGUS=UnfortunatelyYes\n+else\n+\tSNPRINTF_RETURNS_BOGUS=\n+fi\n+AC_SUBST(SNPRINTF_RETURNS_BOGUS)\n+#\n+# Define VSNPRINTF_RETURNS_BOGUS if your are on a system which vsnprintf()\n+# returns -1 instead of number of characters which would have been written\n+# to the final string if enough space had been available.\n+AC_CACHE_CHECK([whether vsnprintf() returns bogus],\n+ [ac_cv_vsnprintf_returns_bogus],\n+[\n+AC_RUN_IFELSE(\n+\t[AC_LANG_PROGRAM([AC_INCLUDES_DEFAULT\n+\t\t#include \"stdarg.h\"\n+\n+\t\tint test_vsnprintf(char *str, size_t maxsize, const char *format, ...)\n+\t\t{\n+\t\t int ret;\n+\t\t va_list ap;\n+\t\t va_start(ap, format);\n+\t\t ret = vsnprintf(str, maxsize, format, ap);\n+\t\t va_end(ap);\n+\t\t return ret;\n+\t\t}],\n+\t\t[[char buf[1];\n+\t\t if (test_vsnprintf(buf, 1, \"%s\", \"12345\") != 5) return 1]])],\n+\t[ac_cv_vsnprintf_returns_bogus=no],\n+\t[ac_cv_vsnprintf_returns_bogus=yes])\n+])\n+if test $ac_cv_vsnprintf_returns_bogus = yes; then\n+\tVSNPRINTF_RETURNS_BOGUS=UnfortunatelyYes\n+else\n+\tVSNPRINTF_RETURNS_BOGUS=\n+fi\n+AC_SUBST(VSNPRINTF_RETURNS_BOGUS)\n \n \n ## Checks for library functions.\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 2a40703..6618c08 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -209,6 +209,18 @@ void *gitmemmem(const void *haystack, size_t haystacklen,\n extern FILE *git_fopen(const char*, const char*);\n #endif\n \n+#ifdef SNPRINTF_RETURNS_BOGUS\n+#define snprintf git_snprintf\n+extern int git_snprintf(char *str, size_t maxsize,\n+ const char *format, ...);\n+#endif\n+\n+#ifdef VSNPRINTF_RETURNS_BOGUS\n+#define vsnprintf git_vsnprintf\n+extern int git_vsnprintf(char *str, size_t maxsize,\n+ const char *format, va_list ap);\n+#endif\n+\n #ifdef __GLIBC_PREREQ\n #if __GLIBC_PREREQ(2, 1)\n #define HAVE_STRCHRNUL\ndiff --git a/dev/null b/compat/snprintf.c\nnew file mode 100644\nindex 0000000..bc0d37c\n--- /dev/null\n+++ b/compat/snprintf.c\n@@ -0,0 +1,37 @@\n+#include \"../git-compat-util.h\"\n+\n+#undef vsnprintf\n+int git_vsnprintf(char *str, size_t maxsize, const char *format, va_list ap)\n+{\n+ char *s;\n+ int size;\n+\n+ int ret = vsnprintf(str, maxsize, format, ap);\n+ if (ret != -1 ) return ret;\n+\n+ s = NULL;\n+ size = maxsize;\n+ while ( ret == -1 )\n+ {\n+ size *= 4;\n+ s = realloc(s, size);\n+ if (! s) return -1;\n+ ret = vsnprintf(s, size, format, ap);\n+ }\n+ if (str && maxsize > 0) memcpy(str, s, maxsize);\n+ free(s);\n+ return ret;\n+}\n+\n+int git_snprintf(char *str, size_t maxsize, const char *format, ...)\n+{\n+ va_list ap;\n+ int ret;\n+\n+ va_start(ap, format);\n+ ret = git_vsnprintf(str, maxsize, format, ap);\n+ va_end(ap);\n+\n+ return ret;\n+}\n+\n\n\n\n-- \nMichal Rokos\n\nNextSoft s.r.o.\nVyskočilova 1/1410\n140 21 Praha 4\nphone: +420 267 224 311\nfax: +420 267 224 307\nmobile: +420 736 646 591\ne-mail: michal.rokos@nextsoft.cz\n"},{"id":"71072","messageId":"200803051458.03593.michal.rokos@nextsoft.cz","threadId":"12493","inReplyTo":"F5DC9F11-FD88-4713-AD1A-6566C345852A@manchester.ac.uk","subject":"Re: [PATCH] Add compat/vsnprintf.c for systems that returns -1 on maxsize reached","fromName":"Michal Rokos","fromEmail":"michal.rokos@nextsoft.cz","sentAt":"2008-03-05T13:58:03Z","receivedAt":"2008-03-05T13:58:03Z","isPatch":true,"sender":{"key":"michal.rokos@nextsoft.cz","avatar":null},"body":"Hello,\n\nOn Wednesday 05 March 2008 11:35:25 Robert Haines wrote:\n> > Could somebody else try to run testcase above on some other OSes?\n\nthank you all for testing on SunOS, Windows, AIX, and MacOS!\n\nMichal\n\n-- \nMichal Rokos\n\nNextSoft s.r.o.\nVyskočilova 1/1410\n140 21 Praha 4\nphone: +420 267 224 311\nfax: +420 267 224 307\nmobile: +420 736 646 591\ne-mail: michal.rokos@nextsoft.cz\n"},{"id":"71076","messageId":"47CEAE20.1030707@viscovery.net","threadId":"12493","inReplyTo":"200803051455.57148.michal.rokos@nextsoft.cz","subject":"Re: [PATCH] Add compat/vsnprintf.c for systems that returns -1 on maxsize reached","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-03-05T14:28:48Z","receivedAt":"2008-03-05T14:28:48Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Michal Rokos schrieb:\n> On Wednesday 05 March 2008 10:18:10 Johannes Sixt wrote:\n>> It's not the same on Windows, which returns:\n>> case1: -1\n>> case2: 5\n>> case3: 5\n>> case4: 5\n>>\n>> BTW, this is not only an issue of vsnprintf, but also of snprintf!\n> \n> Hmm, HPUX has the same issue for snprint() as is for vsnprintf().\n> \n> Do you think that following patch suffices your needs. Please note that it \n> actually copies data to str.\n\n... in the case where the buffer is too small? This won't be a problem for\nour users.\n\n> +# Define SNPRINTF_RETURNS_BOGUS if your are on a system which snprintf()\n> +# returns -1 instead of number of characters which would have been written\n> +# to the final string if enough space had been available.\n> +#\n> +# Define VSNPRINTF_RETURNS_BOGUS if your are on a system which vsnprintf()\n> +# returns -1 instead of number of characters which would have been written\n> +# to the final string if enough space had been available.\n\nWe don't need two configuration variables. I think we can assume that if\nvsnprintf is broken, then snprintf will be broken, too:\n\n# Define SNPRINTF_RETURNS_BOGUS if your are on a system which snprintf()\n# and vsnprintf() return -1 instead of number of characters that would\n# have been written to the final string if enough space had been\n# available.\n\n> +AC_CACHE_CHECK([whether snprintf() returns bogus],\n> + [ac_cv_snprintf_returns_bogus],\n> +[\n> +AC_RUN_IFELSE(\n> +\t[AC_LANG_PROGRAM([AC_INCLUDES_DEFAULT],\n> +\t\t[[char buf[1];\n> +\t\t if (snprintf(bug, 1, \"%s\", \"12345\") != 5) return 1]])],\n ^^^\nbuf?\n\nAre you trying to test for the second bogus behavior on Windows? Don't do\nit! I've thought about it for 5 minutes, but I can't come up with a simple\ntest that would detect its odd behavior.\n\n> diff --git a/dev/null b/compat/snprintf.c\n> new file mode 100644\n> index 0000000..bc0d37c\n> --- /dev/null\n> +++ b/compat/snprintf.c\n> @@ -0,0 +1,37 @@\n> +#include \"../git-compat-util.h\"\n> +\n> +#undef vsnprintf\n> +int git_vsnprintf(char *str, size_t maxsize, const char *format, va_list ap)\n> +{\n> + char *s;\n> + int size;\n> +\n> + int ret = vsnprintf(str, maxsize, format, ap);\n> + if (ret != -1 ) return ret;\n\n'return' goes on its own line. Indentation is one tabstop, not two spaces.\nThank you.\n\n> +\n> + s = NULL;\n\nYou could reuse str here.\n\n> + size = maxsize;\n\nWe are trying to find a suitably long buffer in a loop. We should spend as\nfew cycles as possible. Therefore, my implementation used a minimum of\n250*4 for the first try just in case the caller had a long string to\nconstruct. (And it protects against maxsize == 0.)\n\n> + while ( ret == -1 )\n> + {\n> + size *= 4;\n> + s = realloc(s, size);\n> + if (! s) return -1;\n\nCould you avoid the memory leak on this error path?\n\n> + ret = vsnprintf(s, size, format, ap);\n> + }\n> + if (str && maxsize > 0) memcpy(str, s, maxsize);\n\nWhy this?\n\n> + free(s);\n> + return ret;\n> +}\n\n-- Hannes\n\n"},{"id":"71082","messageId":"200803051600.06605.michal.rokos@nextsoft.cz","threadId":"12493","inReplyTo":"47CEAE20.1030707@viscovery.net","subject":"Re: [PATCH] Add compat/vsnprintf.c for systems that returns -1 on maxsize reached","fromName":"Michal Rokos","fromEmail":"michal.rokos@nextsoft.cz","sentAt":"2008-03-05T15:00:06Z","receivedAt":"2008-03-05T15:00:06Z","isPatch":true,"sender":{"key":"michal.rokos@nextsoft.cz","avatar":null},"body":"Hello,\n\nOn Wednesday 05 March 2008 15:28:48 Johannes Sixt wrote:\n> > Please note that it actually copies data to str.\n>\n> ... in the case where the buffer is too small? This won't be a problem for\n> our users.\n\nDid some more tests. HPUX (v)snprintf() writes chars just well, but returns \nbogus. So no need to copy data explicitly here as well. I'm too quick \non \"Send\" button and slow in testing; sorry.\n\n> > +# Define SNPRINTF_RETURNS_BOGUS if your are on a system which snprintf()\n> > +# returns -1 instead of number of characters which would have been\n> > written +# to the final string if enough space had been available.\n> > +#\n> > +# Define VSNPRINTF_RETURNS_BOGUS if your are on a system which\n> > vsnprintf() +# returns -1 instead of number of characters which would\n> > have been written +# to the final string if enough space had been\n> > available.\n>\n> We don't need two configuration variables. I think we can assume that if\n> vsnprintf is broken, then snprintf will be broken, too:\n\nI don't know. Right now I'm aware of 2 OSes that return bogus - HPUX and \nWindows. I the rest of bugus OSes is the same, 1 config is enough.\n\nThank you for your comments; is this better?\n\ndiff --git a/Makefile b/Makefile\nindex ca5aad9..6af7132 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3,6 +3,10 @@ all::\n \n # Define V=1 to have a more verbose compile.\n #\n+# Define SNPRINTF_RETURNS_BOGUS if your are on a system which snprintf()\n+# or vsnprintf() returns -1 instead of number of characters which would\n+# have been written to the final string if enough space had been available.\n+#\n # Define FREAD_READS_DIRECTORIES if your are on a system which succeeds\n # when attempting to read from an fopen'ed directory.\n #\n@@ -629,6 +633,10 @@ endif\n ifdef NO_C99_FORMAT\n \tBASIC_CFLAGS += -DNO_C99_FORMAT\n endif\n+ifdef SNPRINTF_RETURNS_BOGUS\n+\tCOMPAT_CFLAGS += -DSNPRINTF_RETURNS_BOGUS\n+\tCOMPAT_OBJS += compat/snprintf.o\n+endif\n ifdef FREAD_READS_DIRECTORIES\n \tCOMPAT_CFLAGS += -DFREAD_READS_DIRECTORIES\n \tCOMPAT_OBJS += compat/fopen.o\ndiff --git a/config.mak.in b/config.mak.in\nindex ee6c33d..8e1cd5f 100644\n--- a/config.mak.in\n+++ b/config.mak.in\n@@ -46,3 +46,4 @@ NO_MKDTEMP=@NO_MKDTEMP@\n NO_ICONV=@NO_ICONV@\n OLD_ICONV=@OLD_ICONV@\n NO_DEFLATE_BOUND=@NO_DEFLATE_BOUND@\n+SNPRINTF_RETURNS_BOGUS=@SNPRINTF_RETURNS_BOGUS@\ndiff --git a/configure.ac b/configure.ac\nindex 85d7ef5..a3bbfa6 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -326,6 +326,38 @@ else\n \tNO_C99_FORMAT=\n fi\n AC_SUBST(NO_C99_FORMAT)\n+#\n+# Define SNPRINTF_RETURNS_BOGUS if your are on a system which snprintf()\n+# or vsnprintf() returns -1 instead of number of characters which would\n+# have been written to the final string if enough space had been available.\n+AC_CACHE_CHECK([whether snprintf() and/or vsnprintf() return bogus],\n+ [ac_cv_snprintf_returns_bogus],\n+[\n+AC_RUN_IFELSE(\n+\t[AC_LANG_PROGRAM([AC_INCLUDES_DEFAULT\n+\t\t#include \"stdarg.h\"\n+\n+\t\tint test_vsnprintf(char *str, size_t maxsize, const char *format, ...)\n+\t\t{\n+\t\t int ret;\n+\t\t va_list ap;\n+\t\t va_start(ap, format);\n+\t\t ret = vsnprintf(str, maxsize, format, ap);\n+\t\t va_end(ap);\n+\t\t return ret;\n+\t\t}],\n+\t\t[[char buf[1];\n+\t\t if (test_vsnprintf(buf, 1, \"%s\", \"12345\") != 5) return 1;\n+\t\t if (snprintf(buf, 1, \"%s\", \"12345\") != 5) return 1]])],\n+\t[ac_cv_snprintf_returns_bogus=no],\n+\t[ac_cv_snprintf_returns_bogus=yes])\n+])\n+if test $ac_cv_snprintf_returns_bogus = yes; then\n+\tSNPRINTF_RETURNS_BOGUS=UnfortunatelyYes\n+else\n+\tSNPRINTF_RETURNS_BOGUS=\n+fi\n+AC_SUBST(SNPRINTF_RETURNS_BOGUS)\n \n \n ## Checks for library functions.\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 2a40703..0aa04eb 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -209,6 +209,15 @@ void *gitmemmem(const void *haystack, size_t haystacklen,\n extern FILE *git_fopen(const char*, const char*);\n #endif\n \n+#ifdef SNPRINTF_RETURNS_BOGUS\n+#define snprintf git_snprintf\n+extern int git_snprintf(char *str, size_t maxsize,\n+ const char *format, ...);\n+#define vsnprintf git_vsnprintf\n+extern int git_vsnprintf(char *str, size_t maxsize,\n+ const char *format, va_list ap);\n+#endif\n+\n #ifdef __GLIBC_PREREQ\n #if __GLIBC_PREREQ(2, 1)\n #define HAVE_STRCHRNUL\ndiff --git a/dev/null b/compat/snprintf.c\nnew file mode 100644\nindex 0000000..9f30b22\n--- /dev/null\n+++ b/compat/snprintf.c\n@@ -0,0 +1,39 @@\n+#include \"../git-compat-util.h\"\n+\n+#undef vsnprintf\n+int git_vsnprintf(char *str, size_t maxsize, const char *format, va_list ap)\n+{\n+\tchar *s;\n+\n+\tint ret = vsnprintf(str, maxsize, format, ap);\n+\tif (ret != -1 )\n+\t\treturn ret;\n+\n+\ts = NULL;\n+\n+\twhile (ret == -1) {\n+\t\tmaxsize *= 4;\n+\t\tstr = realloc(s, maxsize);\n+\t\tif (! str) {\n+\t\t\tfree(s);\n+\t\t\treturn -1;\n+\t\t}\n+\t\ts = str;\n+\t\tret = vsnprintf(str, maxsize, format, ap);\n+\t}\n+\tfree(s);\n+\treturn ret;\n+}\n+\n+int git_snprintf(char *str, size_t maxsize, const char *format, ...)\n+{\n+\tva_list ap;\n+\tint ret;\n+\n+\tva_start(ap, format);\n+\tret = git_vsnprintf(str, maxsize, format, ap);\n+\tva_end(ap);\n+\n+\treturn ret;\n+}\n+\n\n-- \nMichal Rokos\n\nNextSoft s.r.o.\nVyskočilova 1/1410\n140 21 Praha 4\nphone: +420 267 224 311\nfax: +420 267 224 307\nmobile: +420 736 646 591\ne-mail: michal.rokos@nextsoft.cz\n"},{"id":"71085","messageId":"47CEBAA3.5060109@viscovery.net","threadId":"12493","inReplyTo":"200803051600.06605.michal.rokos@nextsoft.cz","subject":"Re: [PATCH] Add compat/vsnprintf.c for systems that returns -1 on maxsize reached","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-03-05T15:22:11Z","receivedAt":"2008-03-05T15:22:11Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Michal Rokos schrieb:\n> Thank you for your comments; is this better?\n\nBetter, but still not there. See below. The configure test looks fine, but\nI can't test it.\n\nFinally, please make this a proper patch with Signed-off-by for Junio to\npick up.\n\n> +#undef vsnprintf\n> +int git_vsnprintf(char *str, size_t maxsize, const char *format, va_list ap)\n> +{\n> +\tchar *s;\n> +\n> +\tint ret = vsnprintf(str, maxsize, format, ap);\n> +\tif (ret != -1 )\n> +\t\treturn ret;\n> +\n> +\ts = NULL;\n> +\n\nWhat if maxsize == 0? Insert here:\n\n\tif (maxsize < 250)\n\t\tmaxsize = 250;\n\n> +\twhile (ret == -1) {\n> +\t\tmaxsize *= 4;\n> +\t\tstr = realloc(s, maxsize);\n> +\t\tif (! str) {\n> +\t\t\tfree(s);\n> +\t\t\treturn -1;\n> +\t\t}\n\nret == -1 at this time, so:\n\n\t\tif (!str)\n\t\t\tbreak;\n\nHm?\n\n> +\t\ts = str;\n> +\t\tret = vsnprintf(str, maxsize, format, ap);\n> +\t}\n> +\tfree(s);\n> +\treturn ret;\n> +}\n\n-- Hannes\n\n"},{"id":"71088","messageId":"200803051648.46799.michal.rokos@nextsoft.cz","threadId":"12493","inReplyTo":"47CEBAA3.5060109@viscovery.net","subject":"Re: [PATCH] Add compat/vsnprintf.c for systems that returns -1 on maxsize reached","fromName":"Michal Rokos","fromEmail":"michal.rokos@nextsoft.cz","sentAt":"2008-03-05T15:48:46Z","receivedAt":"2008-03-05T15:48:46Z","isPatch":true,"sender":{"key":"michal.rokos@nextsoft.cz","avatar":null},"body":"Hi,\n\nOn Wednesday 05 March 2008 16:22:11 Johannes Sixt wrote:\n> Michal Rokos schrieb:\n> > Thank you for your comments; is this better?\n>\n> Better, but still not there. See below. The configure test looks fine, but\n> I can't test it.\n>\n> Finally, please make this a proper patch with Signed-off-by for Junio to\n> pick up.\n\nDid that. Thank you for your review.\n\n> What if maxsize == 0? Insert here:\n>\n> \tif (maxsize < 250)\n> \t\tmaxsize = 250;\n\nI've just used 128 which I hope is OK - it's multiplied 4 * anyway.\n\nMichal\n\n-- \nMichal Rokos\n\nNextSoft s.r.o.\nVyskočilova 1/1410\n140 21 Praha 4\nphone: +420 267 224 311\nfax: +420 267 224 307\nmobile: +420 736 646 591\ne-mail: michal.rokos@nextsoft.cz\n"},{"id":"71090","messageId":"20080305155412.GA19295@herod.dreamhost.com","threadId":"12493","inReplyTo":"200803051600.06605.michal.rokos@nextsoft.cz","subject":"Re: [PATCH] Add compat/vsnprintf.c for systems that returns -1 on maxsize reached","fromName":"Wayne Davison","fromEmail":"wayne@opencoder.net","sentAt":"2008-03-05T15:54:12Z","receivedAt":"2008-03-05T15:54:12Z","isPatch":true,"sender":{"key":"wayne@opencoder.net","avatar":"https://gravatar.com/avatar/d55d81825271b1bfe65e57e4e04297d4119aa03c6a1c71d2ff2812b9b4be9f45?d=mp&s=160"},"body":"On Wed, Mar 05, 2008 at 04:00:06PM +0100, Michal Rokos wrote:\n> +\t\t[[char buf[1];\n> +\t\t if (test_vsnprintf(buf, 1, \"%s\", \"12345\") != 5) return 1;\n> +\t\t if (snprintf(buf, 1, \"%s\", \"12345\") != 5) return 1]])],\n\nI'd suggest using a longer buf, requesting a longer length (e.g. 3\ninstead of 1), and then making sure that the resulting buf is right\n(e.g. \"12\" instead of \"123\").\n\n\t\t[[char buf[6];\n\t\t if (test_vsnprintf(buf, 3, \"%s\", \"12345\") != 5\n\t\t || strcmp(buf, \"12\") != 0) return 1;\n\t\t if (snprintf(buf, 3, \"%s\", \"12345\") != 5\n\t\t || strcmp(buf, \"12\") != 0) return 1]])],\n\nThen, set a define that snprintf is bogus and use a version of\nsnprintf() based on this instead:\n\nhttp://rsync.samba.org/ftp/unpacked/rsync/lib/snprintf.c\n\nThat defines rsync_snprintf() and rsync_vsnprintf() functions (which\ncould be renamed for git). Then, in a global .h file, add something\nlike this:\n\n#if !defined HAVE_VSNPRINTF || !defined HAVE_C99_VSNPRINTF\n#define vsnprintf rsync_vsnprintf\nint vsnprintf(char *str, size_t count, const char *fmt, va_list args);\n#endif\n\n#if !defined HAVE_SNPRINTF || !defined HAVE_C99_VSNPRINTF\n#define snprintf rsync_snprintf\nint snprintf(char *str, size_t count, const char *fmt,...);\n#endif\n\nJust be sure to put those that after the various system includes so that\nthey are not adversely affected.\n\n..wayne..\n"},{"id":"71094","messageId":"47CEC47C.6060505@viscovery.net","threadId":"12493","inReplyTo":"20080305155412.GA19295@herod.dreamhost.com","subject":"Re: [PATCH] Add compat/vsnprintf.c for systems that returns -1 on maxsize reached","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-03-05T16:04:12Z","receivedAt":"2008-03-05T16:04:12Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Wayne Davison schrieb:\n> Then, set a define that snprintf is bogus and use a version of\n> snprintf() based on this instead:\n> \n> http://rsync.samba.org/ftp/unpacked/rsync/lib/snprintf.c\n> \n> That defines rsync_snprintf() and rsync_vsnprintf() functions (which\n> could be renamed for git). \n\nNo.\n\nIf one *printf function is replaced, all of them must be replaced. The\nreason is that, eg. on Windows we don't have %llu, but we have %I64u. So\ndepending on which flavor of *printf is called, we would have to supply\nthe one format (rsync_printf: %llu) or the other format (fprintf et al:\n%I64u).\n\n-- Hannes\n"},{"id":"71122","messageId":"7vejaonaoz.fsf@gitster.siamese.dyndns.org","threadId":"12493","inReplyTo":"20080305155412.GA19295@herod.dreamhost.com","subject":"Re: [PATCH] Add compat/vsnprintf.c for systems that returns -1 on maxsize reached","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-05T21:05:32Z","receivedAt":"2008-03-05T21:05:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Wayne Davison writes:\n\n> I'd suggest using a longer buf, requesting a longer length (e.g. 3\n> instead of 1), and then making sure that the resulting buf is right\n> (e.g. \"12\" instead of \"123\").\n>\n> \t\t[[char buf[6];\n> \t\t if (test_vsnprintf(buf, 3, \"%s\", \"12345\") != 5\n> \t\t || strcmp(buf, \"12\") != 0) return 1;\n> \t\t if (snprintf(buf, 3, \"%s\", \"12345\") != 5\n> \t\t || strcmp(buf, \"12\") != 0) return 1]])],\n\nThat sounds like a sensible thing to do.\n\nOther than that, the patch in\n\n Message-Id: <200803051646.13343.michal.rokos@nextsoft.cz>\n\nlooked Ok to me.\n\n"},{"id":"71136","messageId":"20080305223342.GA9620@herod.dreamhost.com","threadId":"12493","inReplyTo":"47CEC47C.6060505@viscovery.net","subject":"Re: [PATCH] Add compat/vsnprintf.c for systems that returns -1 on maxsize reached","fromName":"Wayne Davison","fromEmail":"wayne@opencoder.net","sentAt":"2008-03-05T22:33:42Z","receivedAt":"2008-03-05T22:33:42Z","isPatch":true,"sender":{"key":"wayne@opencoder.net","avatar":"https://gravatar.com/avatar/d55d81825271b1bfe65e57e4e04297d4119aa03c6a1c71d2ff2812b9b4be9f45?d=mp&s=160"},"body":"On Wed, Mar 05, 2008 at 05:04:12PM +0100, Johannes Sixt wrote:\n> If one *printf function is replaced, all of them must be replaced. The\n> reason is that, eg. on Windows we don't have %llu, but we have %I64u.\n\nOr the compatibility function would need to know which escapes to\nemulate (or even emulate a super-set of escapes, as long as they don't\nconflict). If the calling code already has a way to know which escapes\nto use, then the compatibility code could easily be customized to handle\nthe right ones in the same way.\n\nIf the decision is made to not use a compatibility snprintf library,\nthen configure will need a separate test for the limit being off by one,\nand then a fixing function would need to pass through limit-1.\n\n..wayne..\n"},{"id":"71564","messageId":"200803100959.43611.michal.rokos@nextsoft.cz","threadId":"12493","inReplyTo":"47CD78C9.80003@viscovery.net","subject":"Re: [PATCH] Add compat/vsnprintf.c for systems that returns -1 on maxsize reached","fromName":"Michal Rokos","fromEmail":"michal.rokos@nextsoft.cz","sentAt":"2008-03-10T08:59:43Z","receivedAt":"2008-03-10T08:59:43Z","isPatch":true,"sender":{"key":"michal.rokos@nextsoft.cz","avatar":null},"body":"Hello,\n\nI've managed to run testcase on\nTru64 OSF1 V5.1 2650 alpha\nand it returns\ncase1: 0\ncase2: 4\ncase3: 5\ncase4: 5\nwhich is sad since our vsnprintf() cannot detect this ill behaviour.\n\nMichal\n\n-- \nMichal Rokos\n\nNextSoft s.r.o.\nVyskočilova 1/1410\n140 21 Praha 4\nphone: +420 267 224 311\nfax: +420 267 224 307\nmobile: +420 736 646 591\ne-mail: michal.rokos@nextsoft.cz\n"},{"id":"71565","messageId":"47D4FF55.8020909@viscovery.net","threadId":"12493","inReplyTo":"200803100959.43611.michal.rokos@nextsoft.cz","subject":"Re: [PATCH] Add compat/vsnprintf.c for systems that returns -1 on maxsize reached","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-03-10T09:28:53Z","receivedAt":"2008-03-10T09:28:53Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Michal Rokos schrieb:\n> Hello,\n> \n> I've managed to run testcase on\n> Tru64 OSF1 V5.1 2650 alpha\n> and it returns\n> case1: 0\n> case2: 4\n> case3: 5\n> case4: 5\n> which is sad since our vsnprintf() cannot detect this ill behaviour.\n\nUgh! We'd need to run the loop until we get a return value that is not\nnegative and less than the size specified...\n\n-- Hannes\n"},{"id":"71567","messageId":"200803101047.47044.michal.rokos@nextsoft.cz","threadId":"12493","inReplyTo":"47D4FF55.8020909@viscovery.net","subject":"Re: [PATCH] Add compat/vsnprintf.c for systems that returns -1 on maxsize reached","fromName":"Michal Rokos","fromEmail":"michal.rokos@nextsoft.cz","sentAt":"2008-03-10T09:47:46Z","receivedAt":"2008-03-10T09:47:46Z","isPatch":true,"sender":{"key":"michal.rokos@nextsoft.cz","avatar":null},"body":"On Monday 10 March 2008 10:28:53 Johannes Sixt wrote:\n> Michal Rokos schrieb:\n> > Hello,\n> >\n> > I've managed to run testcase on\n> > Tru64 OSF1 V5.1 2650 alpha\n> > and it returns\n> > case1: 0\n> > case2: 4\n> > case3: 5\n> > case4: 5\n> > which is sad since our vsnprintf() cannot detect this ill behaviour.\n>\n> Ugh! We'd need to run the loop until we get a return value that is not\n> negative and less than the size specified...\n\nHmm, I'm afraid we have to grow our buffer size until 2 successive runs return \nthe same value and that value is not -1.\n\nBut - do we want to fix it now, when we don't even know that else would be \nneeded for Tru64 port? There's even no Tru64 paragraph in Makefile yet.\n\nMichal\n\nPS: I have no ambition to port git to Tru64 since that test machine is \naccessible via friend of my friend - so I have to IM him the code and he \nreturns value :)\n\n-- \nMichal Rokos\n\nNextSoft s.r.o.\nVyskočilova 1/1410\n140 21 Praha 4\nphone: +420 267 224 311\nfax: +420 267 224 307\nmobile: +420 736 646 591\ne-mail: michal.rokos@nextsoft.cz\n"},{"id":"71568","messageId":"47D507DA.1010802@viscovery.net","threadId":"12493","inReplyTo":"200803101047.47044.michal.rokos@nextsoft.cz","subject":"Re: [PATCH] Add compat/vsnprintf.c for systems that returns -1 on maxsize reached","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-03-10T10:05:14Z","receivedAt":"2008-03-10T10:05:14Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Michal Rokos schrieb:\n> But - do we want to fix it now, when we don't even know that else would be \n> needed for Tru64 port? There's even no Tru64 paragraph in Makefile yet.\n> \n> Michal\n> \n> PS: I have no ambition to port git to Tru64 since that test machine is \n> accessible via friend of my friend - so I have to IM him the code and he \n> returns value :)\n\nIt's not your itch nor is it not mine. Someone else has to come along...\n\n-- Hannes\n"}]}