<URL: http://bugs.freeciv.org/Ticket/Display.html?id=39328 >

Hi again,

Jason Short wrote:
> <URL: http://bugs.freeciv.org/Ticket/Display.html?id=39328 >
> The current code is clearly wrong.  The va_arg may be implemented as a
> pointer rather than an inline array and so passing it multiple times to
> vsnprintf will generate garbage results on some platforms while working
> on others.

I doubt it matters whether there is an array of varargs structs or a 
linked list of them. With an array, if you use an index too far beyond 
the end, you will also get a segfault. The real problem is that the 
array index or linked list current pointer is not reset to the beginning 
after vsprintf et al. are used.

> Nothing I've read indicates that va_start can be called multiple times
> within the same function, though.

Allow me to requote from the man pages for the GNU libc implementation 
of va_start(3):

"Each invocation of va_start() must be matched by a corresponding 
invocation of va_end() in the same  function.  After
        the call va_end(ap) the variable ap is undefined. Multiple 
transversals of the list, each bracketed by va_start() and
        va_end() are possible."

> So I'm applying the patch as-is.

You apparently applied the original patch and not the modified one that 
I later submitted. The modified one had an additional fix for an 
identical problem in another place in the same source file. That problem 
also caused crashes. Attached to this message is a patch for that problem.


Index: server/plrhand.c
--- server/plrhand.c    (revision 12924)
+++ server/plrhand.c    (working copy)
@@ -768,14 +768,14 @@
   va_list args;
-  va_start(args, format);
   players_iterate(other_player) {
+    va_start(args, format);
     if (!players_on_same_team(pplayer, other_player)) {
     vnotify_conn(other_player->connections, ptile, event, format, args);
+    va_end(args);
   } players_iterate_end;
-  va_end(args);
Freeciv-dev mailing list

Reply via email to