Skip to content

Commit

Permalink
compat/snprintf: don't look at va_list twice
Browse files Browse the repository at this point in the history
If you define SNPRINTF_RETURNS_BOGUS, we use a special
git_vsnprintf wrapper assumes that vsnprintf returns "-1"
instead of the number of characters that you would need to
store the result.

To do this, it invokes vsnprintf multiple times, growing a
heap buffer until we have enough space to hold the result.
However, this means we evaluate the va_list parameter
multiple times, which is generally a bad thing (it may be
modified by calls to vsnprintf, yielding undefined
behavior).

Instead, we must va_copy it and hand the copy to vsnprintf,
so we always have a pristine va_list.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Jeff King authored and Junio C Hamano committed Dec 12, 2011
1 parent 26db0f2 commit a9bfbc5
Showing 1 changed file with 7 additions and 2 deletions.
9 changes: 7 additions & 2 deletions compat/snprintf.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@
#undef vsnprintf
int git_vsnprintf(char *str, size_t maxsize, const char *format, va_list ap)
{
va_list cp;
char *s;
int ret = -1;

if (maxsize > 0) {
ret = vsnprintf(str, maxsize-SNPRINTF_SIZE_CORR, format, ap);
va_copy(cp, ap);
ret = vsnprintf(str, maxsize-SNPRINTF_SIZE_CORR, format, cp);
va_end(cp);
if (ret == maxsize-1)
ret = -1;
/* Windows does not NUL-terminate if result fills buffer */
Expand All @@ -42,7 +45,9 @@ int git_vsnprintf(char *str, size_t maxsize, const char *format, va_list ap)
if (! str)
break;
s = str;
ret = vsnprintf(str, maxsize-SNPRINTF_SIZE_CORR, format, ap);
va_copy(cp, ap);
ret = vsnprintf(str, maxsize-SNPRINTF_SIZE_CORR, format, cp);
va_end(cp);
if (ret == maxsize-1)
ret = -1;
}
Expand Down

0 comments on commit a9bfbc5

Please sign in to comment.