Some platforms provide a horribly broken snprintf. More broken than the
platforms that return -1 when there is too little space in the target buffer
for the formatted string. Some platforms provide an snprintf which _always_
returns the number of characters transmitted to the buffer, regardless of
whether there was enough space or not.
IRIX 6.5 is such a platform. IRIX does have a working snprintf(), but it
is only provided when _NO_XOPEN5 evaluates to zero, and this only happens
if _XOPEN_SOURCE is defined, but definition of _XOPEN_SOURCE prevents
inclusion of many other common functions and defines. So it must be avoided.
Work around these horribly broken snprintf implementations by detecting an
snprintf call which results in the number of transmitted characters exactly
equal to the length of our buffer and retrying with a larger buffer just to
be safe.
Signed-off-by: Brandon Casey <casey@xxxxxxxxxxxxxxx>
---
I was cautious about this change and how it would affect the calling sequence
in strbuf_addf where after an initial failure of vsnprintf, strbuf_grow is
used to increase the strbuf by exactly the number of characters needed and
then vsnprintf is called again.
If strbuf_grow often allocated exactly the number of characters requested,
then the failure code path of git_vsnprintf would be triggered after each
such strbuf_grow. The result of git_vsnprintf would still be successful,
but an unnecessary malloc and vsnprintf would be called.
Thanks to ALLOC_GROW, I don't think it is common for strbuf_grow to allocate
exactly the number of characters that are requested. That is if the test
suite is any indication.
I ran the test suite and counted how many times git_vsnprintf was called,
and how many times the return of the first vsnprintf was equal to maxsize-1.
379056: git_vsnprintf called
2954: ret == maxsize-1 happened (meaning possible failure, but possibly
a buffer exactly the right size)
failure code path .779%
Out of the 379056 times that git_vsnprintf was called during the tests, it
was only 2954 of those times that the initial vsnprintf returned maxsize-1.
That compares to this count on linux (more tests enabled):
405135: git_vsnprintf called
3609: ret >= maxsize (meaning called with buffer too small)
68: ret == maxsize-1 happened (buffer exactly the right size)
failure code path .891%
So, it doesn't seem like it is necessary to complicate this change with more
Makefile options like SNPRINTF_RETURNS_CRAP and then protect the additions
below with preprocessor tests. But that can be done.
-brandon
compat/snprintf.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/compat/snprintf.c b/compat/snprintf.c
index 580966e..357e733 100644
--- a/compat/snprintf.c
+++ b/compat/snprintf.c
@@ -17,6 +17,8 @@ int git_vsnprintf(char *str, size_t maxsize, const char *format, va_list ap)
if (maxsize > 0) {
ret = vsnprintf(str, maxsize-SNPRINTF_SIZE_CORR, format, ap);
+ if (ret == maxsize-1)
+ ret = -1;
/* Windows does not NUL-terminate if result fills buffer */
str[maxsize-1] = 0;
}
@@ -34,6 +36,8 @@ int git_vsnprintf(char *str, size_t maxsize, const char *format, va_list ap)
break;
s = str;
ret = vsnprintf(str, maxsize-SNPRINTF_SIZE_CORR, format, ap);
+ if (ret == maxsize-1)
+ ret = -1;
}
free(s);
return ret;
--
1.6.0.13.ge1c8
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
