Complete.Org: Mailing Lists: Archives: freeciv-dev: May 2005:
[Freeciv-Dev] (PR#13179) Still cannot load any savegame
Home

[Freeciv-Dev] (PR#13179) Still cannot load any savegame

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: mstefek@xxxxxxxxx
Subject: [Freeciv-Dev] (PR#13179) Still cannot load any savegame
From: "Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>
Date: Thu, 26 May 2005 08:19:11 -0700
Reply-to: bugs@xxxxxxxxxxx

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

> [mstefek - Thu May 26 13:24:57 2005]:
> 
> If you run separate server everything seems to load ok.
> 
> The client crashes only if I try to load a savegame using internal 
> server (The "load game" button).
> Assertion is triggered at team.c:69
> pplayer != NULL && pteam != NULL.
> This looks like some kind of client/server synchronization problem in 
> the loading code.

Ouch.

The server sends out player info in the middle of the load.  At this
point some players have no team (teams are assigned to teamless players
at the end).

Receiving a NULL team causes the client to crash.  team_add_player is
called with a NULL team, and it crashes.

When the client crashes the server removes the player.  This causes
team_remove_player to be called on an unteamed player.  Thus the server
also crashes.

This patch fixes both bugs.

-jason

Index: common/team.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/team.c,v
retrieving revision 1.2
diff -u -r1.2 team.c
--- common/team.c       21 May 2005 19:40:24 -0000      1.2
+++ common/team.c       26 May 2005 15:17:24 -0000
@@ -66,32 +66,37 @@
 ****************************************************************************/
 void team_add_player(struct player *pplayer, struct team *pteam)
 {
-  assert(pplayer != NULL && pteam != NULL);
-  assert(&teams[pteam->index] == pteam);
+  assert(pplayer != NULL);
+  assert(!pteam || &teams[pteam->index] == pteam);
 
   /* Remove the player from the old team, if any.  The player's team should
    * only be NULL for a few instants after the player was created; after
    * that they should automatically be put on a team.  So although we
    * check for a NULL case here this is only needed for that one
    * situation. */
-  if (pplayer->team) {
-    team_remove_player(pplayer);
-  }
+  team_remove_player(pplayer);
 
   /* Put the player on the new team. */
   pplayer->team = pteam;
-  pteam->players++;
-  assert(pteam->players <= MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS);
+  if (pteam) {
+    pteam->players++;
+    assert(pteam->players <= MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS);
+  }
 }
 
 /****************************************************************************
   Remove the player from the team.  This should only be called when deleting
   a player; since every player must always be on a team.
+
+  Note in some very rare cases a player may not be on a team.  It's safe
+  to call this function anyway.
 ****************************************************************************/
 void team_remove_player(struct player *pplayer)
 {
-  pplayer->team->players--;
-  assert(pplayer->team->players >= 0);
+  if (pplayer->team) {
+    pplayer->team->players--;
+    assert(pplayer->team->players >= 0);
+  }
   pplayer->team = NULL;
 }
 

[Prev in Thread] Current Thread [Next in Thread]