garrynewman / GWEN

Abandoned: GWEN - GUI Without Extravagant Nonsense.
MIT License
428 stars 102 forks source link

Utility.cpp - error C3861: 'va_copy': identifier not found #44

Open jeremieroy opened 11 years ago

jeremieroy commented 11 years ago

One of the recent commit impacting Utility.cpp break the compatibiltiy with visual studio (at least for 2008) :( The fact is that va_copy is a C99 thing and therefore not present in Visual studio... http://stackoverflow.com/questions/558223/va-copy-porting-to-visual-c

jeremieroy commented 11 years ago

Ok, fix is trivial :

ifdef _MSC_VER

define GWEN_FNULL "NUL"

define va_copy(d,s) ((d) = (s))

else

define GWEN_FNULL "/dev/null"

endif

If someone with a clean branch can push it...

voodooattack commented 11 years ago

Sorry about that one, my bad.

ghost commented 11 years ago

+1, ran into this, the fix worked for me.

hyperknot commented 11 years ago

+1, nice one line fix Can you push it to master?

epajarre commented 11 years ago

(Just updated to a recent Gwen)

I don't like the va_copy fix (It just feels "dirty")

Is there really a need for va_copy?

The current code is;

va_start(s,fmt) { va_list c; va_copy(c,s) actually use c va_end(c) } { va_list c; va_copy(c,s) actually use c va_end(c) } va_end(s)

I think this could be replaced with:

va_start(s,fmt) { just use s directly } va_end(s); va_start(s,fmt); { just use s directly } va_end(s)

No I have not yet actually tested this, because I don't like the fopen("Null_file") idea either.... (but I have used code with does va_start/va_end to same va_list multiple times)

Eero

voodooattack commented 11 years ago

Lack of va_copy() causes segmentation faults on POSIX systems. The Null file trick boosts speed significantly. (especially since the function gets called multiple times per frame)

epajarre commented 11 years ago

Just checking that you did notice that I do a va_end/va_start pair in the middle of my replacement code? (I have been using this kind of code both on Linux and Windows)

Eero

voodooattack commented 11 years ago

Technically, va_start may destroy the argument list as it enumerates it (by popping the stack for example). va_copy is supposed to copy the argument list in order to safely forward it through another call.

Calling va_start repeatedly results in undefined behaviour. It all depends on the compiler/ABI. For example windows allows it, while some systems can't handle it.

The fix posted by jeremieroy appears to work on windows with no overhead (it merely copies a pointer). So I don't see why we should break it again just to make it look nicer on windows.