From d71d37348b38fc7c0d176e374b50581b94a9e579 Mon Sep 17 00:00:00 2001 From: Marko Lindqvist Date: Mon, 10 Jul 2023 01:20:27 +0300 Subject: [PATCH 29/29] Check that team_new() has found a free team slot Also increase MAX_NUM_TEAM_SLOTS by one so there's always space to temporarily have a playerless team when swapping them around. See osdn #48290 Signed-off-by: Marko Lindqvist --- client/packhand.c | 2 ++ common/team.c | 36 ++++++++++++++++++++---------------- common/team.h | 21 +++++++++++++-------- server/plrhand.c | 1 + server/savegame/savegame2.c | 1 + server/savegame/savegame3.c | 1 + server/stdinhand.c | 7 ++++++- 7 files changed, 44 insertions(+), 25 deletions(-) diff --git a/client/packhand.c b/client/packhand.c index db6ce7bfa2..d84071f9eb 100644 --- a/client/packhand.c +++ b/client/packhand.c @@ -2473,6 +2473,8 @@ void handle_player_info(const struct packet_player_info *pinfo) /* Team. */ tslot = team_slot_by_number(pinfo->team); fc_assert(NULL != tslot); + + /* Should never fail when slot given is not nullptr */ team_add_player(pplayer, team_new(tslot)); pnation = nation_by_number(pinfo->nation); diff --git a/common/team.c b/common/team.c index 52a9df5f5f..f0dcc3b5a8 100644 --- a/common/team.c +++ b/common/team.c @@ -318,9 +318,9 @@ struct team *team_new(struct team_slot *tslot) { struct team *pteam; - fc_assert_ret_val(team_slots_initialised(), NULL); + fc_assert_ret_val(team_slots_initialised(), nullptr); - if (NULL == tslot) { + if (tslot == nullptr) { team_slots_iterate(aslot) { if (!team_slot_is_used(aslot)) { tslot = aslot; @@ -328,8 +328,10 @@ struct team *team_new(struct team_slot *tslot) } } team_slots_iterate_end; - fc_assert_ret_val(NULL != tslot, NULL); - } else if (NULL != tslot->team) { + if (tslot == nullptr) { + return nullptr; + } + } else if (tslot->team != nullptr) { return tslot->team; } @@ -356,7 +358,6 @@ void team_destroy(struct team *pteam) struct team_slot *tslot; fc_assert_ret(team_slots_initialised()); - fc_assert_ret(NULL != pteam); fc_assert(0 == player_list_size(pteam->plrlist)); tslot = pteam->slot; @@ -460,22 +461,22 @@ const struct player_list *team_members(const struct team *pteam) } /************************************************************************//** - Set a player to a team. Removes the previous team affiliation if + Set a player to a team. Removes the previous team affiliation if necessary. ****************************************************************************/ -void team_add_player(struct player *pplayer, struct team *pteam) +bool team_add_player(struct player *pplayer, struct team *pteam) { - fc_assert_ret(pplayer != NULL); + fc_assert_ret_val(pplayer != nullptr, FALSE); if (pteam == NULL) { pteam = team_new(NULL); + } else if (pteam == pplayer->team) { + /* It is the team of the player. */ + return TRUE; } - fc_assert_ret(pteam != NULL); - - if (pteam == pplayer->team) { - /* It is the team of the player. */ - return; + if (pteam == nullptr) { + return FALSE; } log_debug("Adding player %d/%s to team %s.", player_number(pplayer), @@ -487,20 +488,22 @@ void team_add_player(struct player *pplayer, struct team *pteam) /* Put the player on the new team. */ pplayer->team = pteam; player_list_append(pteam->plrlist, pplayer); + + return TRUE; } /************************************************************************//** - Remove the player from the team. This should only be called when deleting + 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 + 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) { struct team *pteam; - if (pplayer->team) { + if (pplayer->team != nullptr) { pteam = pplayer->team; log_debug("Removing player %d/%s from team %s (%d)", @@ -512,5 +515,6 @@ void team_remove_player(struct player *pplayer) team_destroy(pteam); } } + pplayer->team = NULL; } diff --git a/common/team.h b/common/team.h index 2aca20f416..2d8f8f61d4 100644 --- a/common/team.h +++ b/common/team.h @@ -1,4 +1,4 @@ -/********************************************************************** +/*********************************************************************** Freeciv - Copyright (C) 2005 - The Freeciv Project This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -18,11 +18,13 @@ extern "C" { #endif /* __cplusplus */ +/* common */ #include "fc_types.h" -#include "tech.h" - -#define MAX_NUM_TEAM_SLOTS MAX_NUM_PLAYER_SLOTS +/* Have +1 to give space to swap teams + * (for the moment when there's one playerless team) + * Affects network protocol and savegames */ +#define MAX_NUM_TEAM_SLOTS (MAX_NUM_PLAYER_SLOTS + 1) /* Opaque types. */ struct team; @@ -50,7 +52,8 @@ void team_slot_set_defined_name(struct team_slot *tslot, const char *team_name); /* Team accessor functions. */ struct team *team_new(struct team_slot *tslot); -void team_destroy(struct team *pteam); +void team_destroy(struct team *pteam) + fc__attribute((nonnull(1))); int team_count(void); int team_index(const struct team *pteam); int team_number(const struct team *pteam); @@ -62,23 +65,25 @@ int team_pretty_name(const struct team *pteam, char *buf, size_t buf_len); const struct player_list *team_members(const struct team *pteam); /* Ancillary routines */ -void team_add_player(struct player *pplayer, struct team *pteam); +bool team_add_player(struct player *pplayer, struct team *pteam); void team_remove_player(struct player *pplayer); -/* iterate over all team slots */ +/* Iterate over all team slots */ #define team_slots_iterate(_tslot) \ if (team_slots_initialised()) { \ struct team_slot *_tslot = team_slot_first(); \ for (; NULL != _tslot; _tslot = team_slot_next(_tslot)) { + #define team_slots_iterate_end \ } \ } -/* iterate over all teams, which are used at the moment */ +/* Iterate over all teams, which are used at the moment */ #define teams_iterate(_pteam) \ team_slots_iterate(_tslot) { \ struct team *_pteam = team_slot_get_team(_tslot); \ if (_pteam != NULL) { + #define teams_iterate_end \ } \ } team_slots_iterate_end; diff --git a/server/plrhand.c b/server/plrhand.c index 282a13b5f4..585d72d2b3 100644 --- a/server/plrhand.c +++ b/server/plrhand.c @@ -1621,6 +1621,7 @@ void server_player_init(struct player *pplayer, bool initmap, } if (needs_team) { team_add_player(pplayer, nullptr); + fc_assert(pplayer->team != nullptr); } /* This must be done after team information is initialised diff --git a/server/savegame/savegame2.c b/server/savegame/savegame2.c index 8905bc9174..e3dfd238c0 100644 --- a/server/savegame/savegame2.c +++ b/server/savegame/savegame2.c @@ -2595,6 +2595,7 @@ static void sg_load_players_basic(struct loaddata *loading) && (tslot = team_slot_by_number(team)), "Invalid team definition for player %s (nb %d).", player_name(pplayer), player_number(pplayer)); + /* Should never fail when slot given is not nullptr */ team_add_player(pplayer, team_new(tslot)); } players_iterate_end; diff --git a/server/savegame/savegame3.c b/server/savegame/savegame3.c index 2235c5d0e8..783abccc89 100644 --- a/server/savegame/savegame3.c +++ b/server/savegame/savegame3.c @@ -3702,6 +3702,7 @@ static void sg_load_players_basic(struct loaddata *loading) && (tslot = team_slot_by_number(team)), "Invalid team definition for player %s (nb %d).", player_name(pplayer), player_number(pplayer)); + /* Should never fail when slot given is not nullptr */ team_add_player(pplayer, team_new(tslot)); } players_iterate_end; diff --git a/server/stdinhand.c b/server/stdinhand.c index fc724e1b87..63e5ba32c0 100644 --- a/server/stdinhand.c +++ b/server/stdinhand.c @@ -2432,10 +2432,11 @@ static bool team_command(struct connection *caller, char *str, bool check) tslot = team_slot_by_number(teamno); } } + if (NULL == tslot) { cmd_reply(CMD_TEAM, caller, C_SYNTAX, _("No such team %s. Please give a " - "valid team name or number."), arg[1]); + "valid team name or number."), arg[1]); goto cleanup; } @@ -2444,19 +2445,23 @@ static bool team_command(struct connection *caller, char *str, bool check) cmd_reply(CMD_TEAM, caller, C_SYNTAX, _("Cannot team a barbarian.")); goto cleanup; } + if (!check) { + /* Should never fail when slot given is not nullptr */ team_add_player(pplayer, team_new(tslot)); send_player_info_c(pplayer, NULL); cmd_reply(CMD_TEAM, caller, C_OK, _("Player %s set to team %s."), player_name(pplayer), team_slot_name_translation(tslot)); } + res = TRUE; cleanup: for (i = 0; i < ntokens; i++) { free(arg[i]); } + return res; } -- 2.40.1