Thread (24 messages) flat view 24 messages, 8 authors, 2016-06-15

Re: [test failure] Re: t4114 binary file becomes symlink

From: Johannes Sixt <hidden>
Date: 2016-06-15 22:47:04

On Sonntag, 19. Juli 2009, Jeff King wrote:
Look at compat/snprintf.c. In git_vsnprintf, we are passed a "va_list
ap", which we then repeatedly call vsnprintf on, checking the return to
make sure we have enough space. But using a va_list repeatedly without a
va_end and va_start in the middle invokes undefined behavior. So we need
to va_copy it and use the copy.

A patch is below, which fixes the problem for me. However, va_copy is
C99, so we would generally try to avoid it. But I don't think there is a
portable way of writing this function without it. And most systems
shouldn't need to use our snprintf at all, so maybe it is portable
enough. I dunno.
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.

-- Hannes
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help