[PATCH 05/11] Remove va_copy at MSVC because there are va_copy.

MSVs have not implemented va_copy. remove va_copy at MSVC environment.
It will malloc buffer each time.

Signed-off-by: Frank Li <redacted>
---
 compat/winansi.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/compat/winansi.c b/compat/winansi.c
index 9217c24..6091138 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c

@@ -3,7 +3,11 @@
  */
 
 #include <windows.h>
+#ifdef _MSC_VER
+#include <stdio.h>
+#else
 #include "../git-compat-util.h"
+#endif
 
 /*
  Functions to be wrapped:

@@ -310,9 +314,13 @@ static int winansi_vfprintf(FILE *stream, const char *format, va_list list)
 	if (!console)
 		goto abort;
 
+#ifndef _MSC_VER 
 	va_copy(cp, list);
 	len = vsnprintf(small_buf, sizeof(small_buf), format, cp);
 	va_end(cp);
+#else
+	len= sizeof(small_buf) ;
+#endif
 
 	if (len > sizeof(small_buf) - 1) {
 		buf = malloc(len + 1);

-- 
1.6.4.msysgit.0

Hi,

On Tue, 18 Aug 2009, Frank Li wrote:

MSVs have not implemented va_copy. remove va_copy at MSVC environment.
It will malloc buffer each time.

Signed-off-by: Frank Li <redacted>

How about this instead?

	Work around Microsoft Visual C++ not having va_copy()

	In winansi.c, Git wants to know the length of the formatted string 
	so it can allocate enough space for it.  But Microsoft Visual C++
	does not have va_copy(), so we have to guess.

The problem is the guessing part:

small_buf only is 256 bytes.  How do you want to make sure that the 
subsequent vsnprintf() is not writing outside of the buffer?

Also, you still miss a space between "len" and "=".

Ciao,
Dscho

On 08/17/2009 06:04 PM, Frank Li wrote:

MSVs have not implemented va_copy. remove va_copy at MSVC environment.
It will malloc buffer each time.

... but only a 257-byte buffer as dscho pointed out.

In many places that do not have va_copy, a simple assignment works.  And 
va_end is almost always a no-op. So what about

#ifndef va_copy
#define va_copy(dst, src)	((dst) = (src))
#endif

if it works on MSVC?

Paolo

2009/8/17 Paolo Bonzini [off-list ref]:

On 08/17/2009 06:04 PM, Frank Li wrote:

quoted

MSVs have not implemented va_copy. remove va_copy at MSVC environment.
It will malloc buffer each time.

... but only a 257-byte buffer as dscho pointed out.

In many places that do not have va_copy, a simple assignment works.  And
va_end is almost always a no-op. So what about

#ifndef va_copy
#define va_copy(dst, src)       ((dst) = (src))
#endif

if it works on MSVC?

According to http://stackoverflow.com/questions/558223/vacopy-porting-to-visual-c
that should work.

- Reece

On Mon, Aug 17, 2009 at 6:53 PM, Paolo Bonzini[off-list ref] wrote:

#ifndef va_copy
#define va_copy(dst, src)       ((dst) = (src))
#endif

Are you sure va_copy is always a preprocessor symbol? How about

#ifdef _MSC_VER
#define va_copy(dst, src)       ((dst) = (src))
#endif

instead? It'd make me sleep slightly better at night, at least ;)

-- 
Erik "kusma" Faye-Lund
kusmabite@gmail.com
(+47) 986 59 656

On Mon, Aug 17, 2009 at 7:02 PM, Erik Faye-Lund[off-list ref] wrote:

Are you sure va_copy is always a preprocessor symbol?

According to the following forum-post we are:
http://www.velocityreviews.com/forums/showpost.php?p=1689162&postcount=2

However, I decided to dig a bit further, so I had a look at the public
draft spec at http://www.open-std.org/JTC1/SC22/WG14/www/docs/n1256.pdf,
section 7.15.1:

"The va_start and va_arg macros described in this subclause shall be implemented
as macros, not functions. It is unspecified whether va_copy and va_end
are macros or
identifiers declared with external linkage."

I don't have access (that I know of) to the finalized spec, but it
looks sketchy to me to depend on va_copy being implemented as a macro
given this wording.

-- 
Erik "kusma" Faye-Lund
kusmabite@gmail.com
(+47) 986 59 656

----- Original Message -----
From: Johannes Schindelin
Date: 8/17/2009 10:49 AM

How about this instead?

	Work around Microsoft Visual C++ not having va_copy()

	In winansi.c, Git wants to know the length of the formatted string 
	so it can allocate enough space for it.  But Microsoft Visual C++
	does not have va_copy(), so we have to guess

I did not look at the surrounding code, but could Microsoft's C runtime 
extension _vscprintf, which returns the number of characters in the 
formatted string, be of use here?

Josh

Hi,

On Mon, 17 Aug 2009, Erik Faye-Lund wrote:

On Mon, Aug 17, 2009 at 6:53 PM, Paolo Bonzini[off-list ref] wrote:

quoted

#ifndef va_copy
#define va_copy(dst, src)       ((dst) = (src))
#endif

Are you sure va_copy is always a preprocessor symbol? How about

#ifdef _MSC_VER
#define va_copy(dst, src)       ((dst) = (src))
#endif

Why not #define it in compat/msvc.h?  Or introduce a 
DEFINE_VA_COPY_TRIVIALLY symbol or some such?

Ciao,
Dscho

#ifndef va_copy
#define va_copy(dst, src)	((dst) = (src))
#endif

if it works on MSVC?

Paolo

I test it, it works.

Hi,

On Tue, 18 Aug 2009, Frank Li wrote:

quoted

#ifndef va_copy
#define va_copy(dst, src)	((dst) = (src))
#endif

if it works on MSVC?

I test it, it works.

But please, either put it into compat/msvc.h or make it dependent on some 
#define such as "DEFINE_VA_COPY_TRIVIALLY" so that other platforms who 
might miss va_copy (but can use the trivial definition above) can use it.  
I do not think that va_copy can be defined like this in general.

Ciao,
Dscho