On Sun, Jul 19, 2009 at 01:01:15PM +0200, Johannes Sixt wrote:
> Problem is, snprintf was made for very old systems, which typically do
> not have va_copy. (E.g. Windows, but there the situation *might* have
> changed with the switch to gcc 4.)
>
> The rationale not to use va_copy is that this function is to be used
> *only* if necessary, i.e. portability is already lacking, and if it
> can be verified that this function works as is. Portability and
> correct-by-the-law C code are *not* a goal here. If the function does
> not work as is, don't use it.
OK, I guess I can buy the "don't use this unless you need it" rationale.
But two questions:
1. _Are_ we sure it works under Windows? That is, do we know for a
fact that using a va_list twice is OK there, or are we just going
on the fact that nobody has reported the bug?
If we're not sure, then you might want to try running the recipe
below which consistently produces a segfault for me on Linux amd64
(but not i386, which seems to use a different representation for
va_lists).
2. In this case, using SNPRINTF_RETURNS_BOGUS was a mistake. But
unfortunately using it erroneously doesn't simply cause the
compilation to barf, or to use a slower implementation; instead it
introduces a very subtle and hard to diagnose bug on some
platforms. Is there anything simple we can do to protect people
from that?
I can't really think of anything simple, because such a mechanism
would basically involve compiling a test program and seeing if it
segfaults.
Anyway, bug-reproducing recipe is below.
-- >8 --
cat <<'EOF' >test-vsnprintf.c
#define SNPRINTF_RETURNS_BOGUS
#include "git-compat-util.h"
int main() {
char buf[16];
/* this 8 may need to be tweaked depending on
* the system's vsnprintf return value; the goal
* is to get git_vsnprintf to have to look at
* it's va_list twice */
git_snprintf(buf, 8, "%s %s", "foo", "bar");
return 0;
}
EOF
make test-vsnprintf.o compat/snprintf.o
gcc -o test-vsnprintf test-vsnprintf.o compat/snprintf.o
./test-vsnprintf ;# or valgrind test-vsnprintf
--
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