<URL: http://bugs.freeciv.org/Ticket/Display.html?id=40033 >
Some helpful debugging improvements, and other niceties found along the way:
* sanity checking with file and line
* clear city and unit pointers on free()
* always pair create_unit_virtual() with destroy_unit_virtual()
* moved kill_dying_players() out of player packet handling (plrhand.c)
into server main (srv_main.c), the only place it's called now (3 times).
* for MacOSX, die() doesn't give a traceback. Fake a bad memory access.
Only in nation_of_player(). Probably should do this elsewhere, too.
Index: server/srv_main.c
===================================================================
--- server/srv_main.c (revision 14266)
+++ server/srv_main.c (working copy)
@@ -557,6 +557,33 @@
} players_iterate_end;
}
+/****************************************************************************
+ Check all players to see whether they are dying.
+
+ WARNING: do not call this while doing any handling of players, units,
+ etc. If a player dies, all his units will be wiped and other data will
+ be overwritten.
+
+ FIXME: merge is_alive (105) with is_dying (8) and surrendered (7)?
+****************************************************************************/
+static void kill_dying_players(void)
+{
+ players_iterate(pplayer) {
+ if (pplayer->is_alive) {
+ /* cities or units remain? */
+ if (0 == city_list_size(pplayer->cities)
+ && 0 == unit_list_size(pplayer->units)) {
+ pplayer->is_dying = TRUE;
+ }
+ /* also F_GAMELOSS in unittools server_remove_unit() */
+ if (pplayer->is_dying) {
+ pplayer->is_dying = FALSE; /* Can't get more dead than this. */
+ kill_player(pplayer);
+ }
+ }
+ } players_iterate_end;
+}
+
/**************************************************************************
Called at the start of each (new) phase to do AI activities.
**************************************************************************/
Index: server/settlers.c
===================================================================
--- server/settlers.c (revision 14266)
+++ server/settlers.c (working copy)
@@ -1343,7 +1343,7 @@
-result.result : result.result);
pcity->ai.founder_boat = result.overseas;
}
- free(virtualunit);
+ destroy_unit_virtual(virtualunit);
}
/**************************************************************************
@@ -1377,7 +1377,7 @@
NULL);
want = (want - unit_food_upkeep(virtualunit) * FOOD_WEIGHTING) * 100
/ (40 + unit_foodbox_cost(virtualunit));
- free(virtualunit);
+ destroy_unit_virtual(virtualunit);
/* Massage our desire based on available statistics to prevent
* overflooding with worker type units if they come cheap in
Index: server/sanitycheck.c
===================================================================
--- server/sanitycheck.c (revision 14266)
+++ server/sanitycheck.c (working copy)
@@ -25,41 +25,47 @@
#include "map.h"
#include "movement.h"
#include "player.h"
+#include "specialist.h"
#include "terrain.h"
#include "unit.h"
#include "unitlist.h"
#include "citytools.h"
#include "maphand.h"
-#include "sanitycheck.h"
+#include "srv_main.h"
#include "unittools.h"
+#include "sanitycheck.h"
+
#ifdef SANITY_CHECKING
#define SANITY_CHECK(x)
\
do { \
if (!(x)) {
\
- freelog(LOG_ERROR, "Failed sanity check: %s (%s:%d)", \
- #x, __FILE__,__LINE__); \
+ freelog(LOG_ERROR, "Failed %s:%d (%s:%d): %s", \
+ __FILE__,__LINE__, file, line, #x); \
} \
} while(0)
#define SANITY_TILE(ptile, check) \
do { \
if (!(check)) { \
- freelog(LOG_ERROR, "Failed sanity check at %s (%d, %d): "
\
- "%s (%s:%d)", ptile->city ? city_name(ptile->city) \
- : ptile->terrain->name.vernacular, TILE_XY(ptile), \
- #check, __FILE__,__LINE__); \
+ struct city *pcity = tile_get_city(ptile); \
+ freelog(LOG_ERROR, "Failed %s:%d (%s:%d) at %s (%d,%d): %s", \
+ __FILE__,__LINE__, file, line, \
+ NULL != pcity ? city_name(pcity) \
+ : terrain_rule_name(tile_get_terrain(ptile)), \
+ TILE_XY(ptile), #check); \
} \
} while(0)
#define SANITY_CITY(pcity, check) \
do { \
if (!(check)) { \
- freelog(LOG_ERROR, "Failed sanity check in %s[%d](%d, %d): " \
- "%s (%s:%d)", city_name(pcity), pcity->size, \
- TILE_XY(pcity->tile), #check, __FILE__,__LINE__); \
+ freelog(LOG_ERROR, "Failed %s:%d (%s:%d) in %s[%d](%d,%d): %s", \
+ __FILE__,__LINE__, file, line, \
+ city_name(pcity), (pcity)->size, \
+ TILE_XY((pcity)->tile), #check); \
} \
} while(0)
@@ -67,7 +73,7 @@
/**************************************************************************
Sanity checking on map (tile) specials.
**************************************************************************/
-static void check_specials(void)
+static void check_specials(const char *file, int line)
{
whole_map_iterate(ptile) {
const struct terrain *pterrain = tile_get_terrain(ptile);
@@ -93,7 +99,7 @@
/**************************************************************************
Sanity checking on fog-of-war (visibility, shared vision, etc.).
**************************************************************************/
-static void check_fow(void)
+static void check_fow(const char *file, int line)
{
whole_map_iterate(ptile) {
players_iterate(pplayer) {
@@ -109,9 +115,9 @@
* defined. */
if (game.info.fogofwar) {
if (plr_tile->seen_count[v] > 0) {
- SANITY_TILE(ptile, BV_ISSET(ptile->tile_seen[v],
pplayer->player_no));
+ SANITY_TILE(ptile, BV_ISSET(ptile->tile_seen[v],
player_index(pplayer)));
} else {
- SANITY_TILE(ptile, !BV_ISSET(ptile->tile_seen[v],
pplayer->player_no));
+ SANITY_TILE(ptile, !BV_ISSET(ptile->tile_seen[v],
player_index(pplayer)));
}
}
@@ -134,7 +140,7 @@
/**************************************************************************
Miscellaneous sanity checks.
**************************************************************************/
-static void check_misc(void)
+static void check_misc(const char *file, int line)
{
int nbarbs = 0;
players_iterate(pplayer) {
@@ -151,7 +157,7 @@
/**************************************************************************
Sanity checks on the map itself. See also check_specials.
**************************************************************************/
-static void check_map(void)
+static void check_map(const char *file, int line)
{
whole_map_iterate(ptile) {
struct city *pcity = tile_get_city(ptile);
@@ -161,7 +167,7 @@
CHECK_MAP_POS(ptile->x, ptile->y);
CHECK_NATIVE_POS(ptile->nat_x, ptile->nat_y);
- if (ptile->city) {
+ if (NULL != pcity) {
SANITY_TILE(ptile, tile_owner(ptile) != NULL);
}
if (tile_owner(ptile) != NULL) {
@@ -344,13 +350,13 @@
/**************************************************************************
Sanity checks on all cities in the world.
**************************************************************************/
-static void check_cities(void)
+static void check_cities(const char *file, int line)
{
players_iterate(pplayer) {
city_list_iterate(pplayer->cities, pcity) {
SANITY_CITY(pcity, city_owner(pcity) == pplayer);
- sanity_check_city(pcity);
+ real_sanity_check_city(pcity, file, line);
} city_list_iterate_end;
} players_iterate_end;
@@ -378,7 +384,7 @@
/**************************************************************************
Sanity checks on all units in the world.
**************************************************************************/
-static void check_units(void) {
+static void check_units(const char *file, int line) {
players_iterate(pplayer) {
unit_list_iterate(pplayer->units, punit) {
struct tile *ptile = punit->tile;
@@ -446,11 +452,12 @@
/**************************************************************************
Sanity checks on all players.
**************************************************************************/
-static void check_players(void)
+static void check_players(const char *file, int line)
{
int player_no;
players_iterate(pplayer) {
+ int one = player_index(pplayer);
int found_palace = 0;
if (!pplayer->is_alive) {
@@ -467,11 +474,12 @@
} city_list_iterate_end;
players_iterate(pplayer2) {
- SANITY_CHECK(pplayer->diplstates[pplayer2->player_no].type
- == pplayer2->diplstates[pplayer->player_no].type);
- if (pplayer->diplstates[pplayer2->player_no].type == DS_CEASEFIRE) {
- SANITY_CHECK(pplayer->diplstates[pplayer2->player_no].turns_left
- == pplayer2->diplstates[pplayer->player_no].turns_left);
+ int two = player_index(pplayer2);
+ SANITY_CHECK(pplayer->diplstates[two].type
+ == pplayer2->diplstates[one].type);
+ if (pplayer->diplstates[two].type == DS_CEASEFIRE) {
+ SANITY_CHECK(pplayer->diplstates[two].turns_left
+ == pplayer2->diplstates[one].turns_left);
}
if (pplayer->is_alive
&& pplayer2->is_alive
@@ -517,7 +525,7 @@
/****************************************************************************
Sanity checking on teams.
****************************************************************************/
-static void check_teams(void)
+static void check_teams(const char *file, int line)
{
int count[MAX_NUM_TEAMS], i;
@@ -538,7 +546,7 @@
/**************************************************************************
Sanity checking on connections.
**************************************************************************/
-static void check_connections(void)
+static void check_connections(const char *file, int line)
{
/* est_connections is a subset of all_connections */
SANITY_CHECK(conn_list_size(game.all_connections)
@@ -554,21 +562,21 @@
can't call it in the middle of an operation that is supposed to be
atomic.
**************************************************************************/
-void sanity_check(void)
+void real_sanity_check(const char *file, int line)
{
if (!map_is_empty()) {
/* Don't sanity-check the map if it hasn't been created yet (this
* happens when loading scenarios). */
- check_specials();
- check_map();
- check_cities();
- check_units();
- check_fow();
+ check_specials(file, line);
+ check_map(file, line);
+ check_cities(file, line);
+ check_units(file, line);
+ check_fow(file, line);
}
- check_misc();
- check_players();
- check_teams();
- check_connections();
+ check_misc(file, line);
+ check_players(file, line);
+ check_teams(file, line);
+ check_connections(file, line);
}
#endif /* SANITY_CHECKING */
Index: server/sanitycheck.h
===================================================================
--- server/sanitycheck.h (revision 14266)
+++ server/sanitycheck.h (working copy)
@@ -25,7 +25,8 @@
# define sanity_check_city(x) real_sanity_check_city(x, __FILE__, __LINE__)
void real_sanity_check_city(struct city *pcity, const char *file, int line);
-void sanity_check(void);
+# define sanity_check() real_sanity_check(__FILE__, __LINE__)
+void real_sanity_check(const char *file, int line);
#else /* SANITY_CHECKING */
Index: server/plrhand.c
===================================================================
--- server/plrhand.c (revision 14266)
+++ server/plrhand.c (working copy)
@@ -97,36 +97,15 @@
send_global_city_turn_notifications(dest);
}
-/****************************************************************************
- Check all players to see if they are dying. Kill them if so.
-
- WARNING: do not call this while doing any handling of players, units,
- etc. If a player dies, all his units will be wiped and other data will
- be overwritten.
-****************************************************************************/
-void kill_dying_players(void)
-{
- players_iterate(pplayer) {
- if (pplayer->is_alive) {
- if (unit_list_size(pplayer->units) == 0
- && city_list_size(pplayer->cities) == 0) {
- pplayer->is_dying = TRUE;
- }
- if (pplayer->is_dying) {
- kill_player(pplayer);
- }
- }
- } players_iterate_end;
-}
-
/**************************************************************************
Murder a player in cold blood.
+
+ Called only from srv_main kill_dying_players()
**************************************************************************/
void kill_player(struct player *pplayer)
{
bool palace;
- pplayer->is_dying = FALSE; /* Can't get more dead than this. */
pplayer->is_alive = FALSE;
/* Remove shared vision from dead player to friends. */
@@ -1146,7 +1125,8 @@
void make_contact(struct player *pplayer1, struct player *pplayer2,
struct tile *ptile)
{
- int player1 = player_number(pplayer1), player2 = player_number(pplayer2);
+ int player1 = player_index(pplayer1);
+ int player2 = player_index(pplayer2);
if (pplayer1 == pplayer2
|| !pplayer1->is_alive
Index: server/plrhand.h
===================================================================
--- server/plrhand.h (revision 14266)
+++ server/plrhand.h (working copy)
@@ -33,7 +33,6 @@
bool initmap, bool needs_team);
void server_remove_player(struct player *pplayer);
void kill_player(struct player *pplayer);
-void kill_dying_players(void);
void update_revolution(struct player *pplayer);
struct nation_type *pick_a_nation(struct nation_type **choices,
Index: common/unit.c
===================================================================
--- common/unit.c (revision 14266)
+++ common/unit.c (working copy)
@@ -1454,6 +1454,7 @@
void destroy_unit_virtual(struct unit *punit)
{
free_unit_orders(punit);
+ memset(punit, 0, sizeof(*punit)); /* ensure no pointers remain */
free(punit);
}
Index: common/city.c
===================================================================
--- common/city.c (revision 14266)
+++ common/city.c (working copy)
@@ -2526,5 +2526,6 @@
void remove_city_virtual(struct city *pcity)
{
unit_list_free(pcity->units_supported);
+ memset(pcity, 0, sizeof(*pcity)); /* ensure no pointers remain */
free(pcity);
}
Index: common/game.c
===================================================================
--- common/game.c (revision 14266)
+++ common/game.c (working copy)
@@ -138,9 +138,7 @@
pcity = player_find_city_by_id(unit_owner(punit), punit->homecity);
if (pcity) {
unit_list_unlink(pcity->units_supported, punit);
- }
- if (pcity) {
freelog(LOG_DEBUG, "home city %s, %s, (%d %d)",
city_name(pcity),
nation_rule_name(nation_of_city(pcity)),
Index: common/nation.c
===================================================================
--- common/nation.c (revision 14266)
+++ common/nation.c (working copy)
@@ -252,13 +252,18 @@
/****************************************************************************
Return the nation of a player.
****************************************************************************/
-struct nation_type *nation_of_player(const struct player *plr)
+struct nation_type *nation_of_player(const struct player *pplayer)
{
- assert(plr != NULL);
- if (!bounds_check_nation(plr->nation, LOG_FATAL, "nation_of_player")) {
+ assert(NULL != pplayer);
+ if (!bounds_check_nation(pplayer->nation, LOG_FATAL, "nation_of_player")) {
+#if defined(__APPLE__)
+ /* force trace log */
+ return ((struct nation_type **)pplayer)[0];
+#else
die("bad nation");
+#endif
}
- return plr->nation;
+ return pplayer->nation;
}
/****************************************************************************
Index: client/agents/sha.c
===================================================================
--- client/agents/sha.c (revision 14266)
+++ client/agents/sha.c (working copy)
@@ -89,6 +89,9 @@
assert(pold_unit);
unit_list_unlink(previous_units, pold_unit);
+ /* list pointers were struct copied, cannot destroy_unit_virtual() */
+ memset(pold_unit, 0, sizeof(*pold_unit)); /* ensure no pointers remain */
+ free(pold_unit);
}
/**************************************************************************
_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
