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

 If there is no free player slots, detached connection doing "take -"
crashes server.

 To reproduce
 S2_1:
   1. Connect client A to server
   2. A: /set aifill 30
   3. Connect client B to server (connection is not automatically attached)
   4. B: /take -

 S2_2 and TRUNK:
   1. Connect client A to server
   2. A: /set maxplayers 1
   3. Connect client B to server (connection is not automatically attached)
   4. B: /take -


 Fix attached. I'll commit ASAP.



 - ML

diff -Nurd -X.diff_ignore freeciv/server/connecthand.c freeciv/server/connecthand.c
--- freeciv/server/connecthand.c	2008-01-15 04:04:13.000000000 +0200
+++ freeciv/server/connecthand.c	2008-04-01 23:25:36.000000000 +0300
@@ -448,6 +448,9 @@
   associated.
   Note "observer" connections do not count for is_connected. You must set
        pconn->observer to TRUE before attaching!
+  Note take_command() needs to know if this function will success before
+       it's time to call this. Keep take_command() checks in sync when
+       modifying this.
 **************************************************************************/
 bool attach_connection_to_player(struct connection *pconn,
                                  struct player *pplayer)
@@ -456,7 +459,7 @@
   if (!pplayer) {
     if (game.info.nplayers >= game.info.max_players 
         || game.info.nplayers >= MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS) {
-      return FALSE; 
+      return FALSE;
     } else {
       pplayer = &game.players[game.info.nplayers];
       server_player_init(pplayer, FALSE, TRUE);
diff -Nurd -X.diff_ignore freeciv/server/stdinhand.c freeciv/server/stdinhand.c
--- freeciv/server/stdinhand.c	2008-04-01 10:37:54.000000000 +0300
+++ freeciv/server/stdinhand.c	2008-04-01 23:26:51.000000000 +0300
@@ -2952,6 +2952,19 @@
     goto end;
   }
 
+  /* Make sure there is free player slot if new player is
+   * requested by "take -". This is necessary for previously
+   * detached connections only. Others can reuse the slot
+   * they first release. */
+  if (!pplayer && !pconn->player
+      && (game.info.nplayers >= game.info.max_players
+          || game.info.nplayers >= MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS)) {
+    cmd_reply(CMD_TAKE, caller, C_FAIL,
+              _("There is no free player slot for %s"),
+              pconn->username);
+    goto end;
+  }
+
   res = TRUE;
   if (check) {
     goto end;
@@ -3012,17 +3025,22 @@
 
   /* now attach to new player */
   pconn->observer = FALSE; /* do this before attach! */
-  attach_connection_to_player(pconn, pplayer);
-  pplayer = pconn->player; /* In case pplayer was NULL. */
- 
-  /* if pplayer wasn't /created, and we're still in pregame, change its name */
-  if (!pplayer->was_created && is_newgame && !pplayer->nation) {
-    sz_strlcpy(pplayer->name, pconn->username);
-  }
+  res = attach_connection_to_player(pconn, pplayer);
 
-  /* aitoggle the player back to human as necessary. */
-  if (pplayer->ai.control && game.info.auto_ai_toggle) {
-    toggle_ai_player_direct(NULL, pplayer);
+  if (res) {
+    /* Successfully attached */
+    pplayer = pconn->player; /* In case pplayer was NULL. */
+
+    /* if pplayer wasn't /created, and we're still in pregame,
+       change its name */
+    if (!pplayer->was_created && is_newgame && !pplayer->nation) {
+      sz_strlcpy(pplayer->name, pconn->username);
+    }
+
+    /* aitoggle the player back to human as necessary. */
+    if (pplayer->ai.control && game.info.auto_ai_toggle) {
+      toggle_ai_player_direct(NULL, pplayer);
+    }
   }
 
   if (S_S_RUNNING == server_state()) {
@@ -3041,17 +3059,22 @@
   /* redundant self to self cannot be avoided */
   send_conn_info(pconn->self, game.est_connections);
 
-  cmd_reply(CMD_TAKE, caller, C_OK, _("%s now controls %s (%s, %s)"), 
-            pconn->username,
-            player_name(pplayer), 
-            is_barbarian(pplayer)
-            ? _("Barbarian")
-            : pplayer->ai.control
+  if (res) {
+    cmd_reply(CMD_TAKE, caller, C_OK, _("%s now controls %s (%s, %s)"), 
+              pconn->username,
+              player_name(pplayer), 
+              is_barbarian(pplayer)
+              ? _("Barbarian")
+              : pplayer->ai.control
               ? _("AI")
               : _("Human"),
-            pplayer->is_alive
-            ? _("Alive")
-            : _("Dead"));
+              pplayer->is_alive
+              ? _("Alive")
+              : _("Dead"));
+  } else {
+    cmd_reply(CMD_TAKE, caller, C_FAIL, _("%s failed to attach to any player"),
+              pconn->username);
+  }
 
   end:;
   /* free our args */
_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

Reply via email to