Re: [Freeciv-Dev] (PR#39511) [Patch] Call init_available_nations()
URL: http://bugs.freeciv.org/Ticket/Display.html?id=39511
On 10/08/07, Marko Lindqvist [EMAIL PROTECTED] wrote:
On 08/08/07, Marko Lindqvist [EMAIL PROTECTED] wrote:
There was another call of init_available_nations() at wrong time. And
sometimes it was not called when it should have been called.
- Fixed client crash
Separate version for S2_1. New asserts should go to trunk only.
- ML
diff -Nurd -X.diff_ignore freeciv/server/ruleset.c freeciv/server/ruleset.c
--- freeciv/server/ruleset.c 2007-08-07 16:49:47.0 +0300
+++ freeciv/server/ruleset.c 2007-08-11 21:29:12.0 +0300
@@ -44,6 +44,7 @@
#include aiunit.h /* update_simple_ai_types */
#include ruleset.h
+#include srv_main.h
/* RULESET_SUFFIX already used, no leading dot here */
#define RULES_SUFFIX ruleset
@@ -3132,6 +3133,10 @@
static void reset_player_nations(void)
{
players_iterate(pplayer) {
+/* We cannot use player_set_nation() here since this is
+ * called before nations are loaded. Player pointers
+ * from nations will be initialized to NULL when nations are
+ * loaded. */
pplayer-nation = NO_NATION_SELECTED;
pplayer-city_style = 0;
} players_iterate_end;
@@ -3186,6 +3191,9 @@
load_ruleset_effects(effectfile);
load_ruleset_game();
+ /* Init nations we just loaded. */
+ init_available_nations();
+
sanity_check_ruleset_data();
precalc_tech_data();
diff -Nurd -X.diff_ignore freeciv/server/srv_main.c freeciv/server/srv_main.c
--- freeciv/server/srv_main.c 2007-08-11 12:17:10.0 +0300
+++ freeciv/server/srv_main.c 2007-08-11 21:32:32.0 +0300
@@ -1307,7 +1307,24 @@
}
}
nations_iterate(nation) {
-nation-player = NULL;
+/* Even though this function is called init_available_nations(),
+ * nation-player should never have value assigned to it
+ * (since it has beeen initialized in load_rulesets() ). */
+if (nation-player != NULL) {
+
+ freelog(LOG_ERROR, Player assigned to nation before
+ init_available_nations());
+
+ /* Try to handle error situation as well as we can */
+ if (nation-player-nation == nation) {
+/* At least assignment is consistent. Leave nation assigned,
+ * and make sure that nation is also marked available. */
+nation-is_available = TRUE;
+ } else {
+/* Not consistent. Just initialize the pointer and hope for the best */
+nation-player = NULL;
+ }
+}
} nations_iterate_end;
send_ruleset_nations(game.est_connections);
}
@@ -1776,7 +1793,6 @@
con_flush();
- server_game_init();
stdinhand_init();
diplhand_init();
@@ -1786,6 +1802,8 @@
server_open_socket();
}
+ server_game_init();
+
/* load a saved game */
if (srvarg.load_filename[0] != '\0') {
(void) load_command(NULL, srvarg.load_filename, FALSE);
@@ -1874,8 +1892,6 @@
**/
static void srv_loop(void)
{
- init_available_nations();
-
freelog(LOG_NORMAL, _(Now accepting new client connections.));
while(server_state == PRE_GAME_STATE) {
sniff_packets(); /* Accepting commands. */
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39511) [Patch] Call init_available_nations()
URL: http://bugs.freeciv.org/Ticket/Display.html?id=39511
On 11/08/07, Marko Lindqvist [EMAIL PROTECTED] wrote:
New asserts should go to trunk only.
Actually there is easily reproducible bug causing second assert to
fail. At this point I will commit this patch without that assert.
Assert should be added when known bug is fixed.
- ML
diff -Nurd -X.diff_ignore freeciv/server/ruleset.c freeciv/server/ruleset.c
--- freeciv/server/ruleset.c 2007-08-09 17:20:08.0 +0300
+++ freeciv/server/ruleset.c 2007-08-11 21:43:12.0 +0300
@@ -46,6 +46,7 @@
#include aiunit.h /* update_simple_ai_types */
#include ruleset.h
+#include srv_main.h
/* RULESET_SUFFIX already used, no leading dot here */
#define RULES_SUFFIX ruleset
@@ -3494,6 +3495,10 @@
static void reset_player_nations(void)
{
players_iterate(pplayer) {
+/* We cannot use player_set_nation() here since this is
+ * called before nations are loaded. Player pointers
+ * from nations will be initialized to NULL when nations are
+ * loaded. */
pplayer-nation = NO_NATION_SELECTED;
pplayer-city_style = 0;
} players_iterate_end;
@@ -3548,6 +3553,9 @@
load_ruleset_effects(effectfile);
load_ruleset_game();
+ /* Init nations we just loaded. */
+ init_available_nations();
+
sanity_check_ruleset_data();
precalc_tech_data();
diff -Nurd -X.diff_ignore freeciv/server/srv_main.c freeciv/server/srv_main.c
--- freeciv/server/srv_main.c 2007-08-10 03:05:02.0 +0300
+++ freeciv/server/srv_main.c 2007-08-11 21:43:12.0 +0300
@@ -1347,7 +1347,29 @@
}
}
nations_iterate(nation) {
-nation-player = NULL;
+/* Even though this function is called init_available_nations(),
+ * nation-player should never have value assigned to it
+ * (since it has beeen initialized in load_rulesets() ). */
+if (nation-player != NULL) {
+
+ freelog(LOG_ERROR, Player assigned to nation before
+ init_available_nations());
+
+ /* When we enter this execution branch, assert() will always
+ * fail. This one just provides more informative message than
+ * simple assert(FAIL); */
+ assert(nation-player == NULL);
+
+ /* Try to handle error situation as well as we can */
+ if (nation-player-nation == nation) {
+/* At least assignment is consistent. Leave nation assigned,
+ * and make sure that nation is also marked available. */
+nation-is_available = TRUE;
+ } else {
+/* Not consistent. Just initialize the pointer and hope for the best */
+nation-player = NULL;
+ }
+}
} nations_iterate_end;
send_ruleset_nations(game.est_connections);
}
@@ -1791,7 +1813,6 @@
con_flush();
- server_game_init();
stdinhand_init();
diplhand_init();
@@ -1801,6 +1822,8 @@
server_open_socket();
}
+ server_game_init();
+
/* load a saved game */
if (srvarg.load_filename[0] != '\0') {
(void) load_command(NULL, srvarg.load_filename, FALSE);
@@ -1889,8 +1912,6 @@
**/
static void srv_loop(void)
{
- init_available_nations();
-
freelog(LOG_NORMAL, _(Now accepting new client connections.));
while(server_state == PRE_GAME_STATE) {
sniff_packets(); /* Accepting commands. */
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39511) [Patch] Call init_available_nations()
URL: http://bugs.freeciv.org/Ticket/Display.html?id=39511
On 08/08/07, Marko Lindqvist [EMAIL PROTECTED] wrote:
There was another call of init_available_nations() at wrong time. And
sometimes it was not called when it should have been called.
- Fixed client crash
- ML
diff -Nurd -X.diff_ignore freeciv/common/player.c freeciv/common/player.c
--- freeciv/common/player.c 2007-08-04 18:38:32.0 +0300
+++ freeciv/common/player.c 2007-08-10 19:35:51.0 +0300
@@ -330,6 +330,11 @@
}
pplayer-nation = pnation;
return TRUE;
+ } else {
+/* Nation already assigned to this player.
+ * Or there was - nor will be - nation for player. */
+assert(pnation == NULL
+ || pnation-player == pplayer);
}
return FALSE;
}
diff -Nurd -X.diff_ignore freeciv/server/ruleset.c freeciv/server/ruleset.c
--- freeciv/server/ruleset.c 2007-08-09 17:20:08.0 +0300
+++ freeciv/server/ruleset.c 2007-08-10 19:26:40.0 +0300
@@ -46,6 +46,7 @@
#include aiunit.h /* update_simple_ai_types */
#include ruleset.h
+#include srv_main.h
/* RULESET_SUFFIX already used, no leading dot here */
#define RULES_SUFFIX ruleset
@@ -3494,6 +3495,10 @@
static void reset_player_nations(void)
{
players_iterate(pplayer) {
+/* We cannot use player_set_nation() here since this is
+ * called before nations are loaded. Player pointers
+ * from nations will be initialized to NULL when nations are
+ * loaded. */
pplayer-nation = NO_NATION_SELECTED;
pplayer-city_style = 0;
} players_iterate_end;
@@ -3548,6 +3553,9 @@
load_ruleset_effects(effectfile);
load_ruleset_game();
+ /* Init nations we just loaded. */
+ init_available_nations();
+
sanity_check_ruleset_data();
precalc_tech_data();
diff -Nurd -X.diff_ignore freeciv/server/srv_main.c freeciv/server/srv_main.c
--- freeciv/server/srv_main.c 2007-08-10 03:05:02.0 +0300
+++ freeciv/server/srv_main.c 2007-08-10 19:26:40.0 +0300
@@ -1347,7 +1347,29 @@
}
}
nations_iterate(nation) {
-nation-player = NULL;
+/* Even though this function is called init_available_nations(),
+ * nation-player should never have value assigned to it
+ * (since it has beeen initialized in load_rulesets() ). */
+if (nation-player != NULL) {
+
+ freelog(LOG_ERROR, Player assigned to nation before
+ init_available_nations());
+
+ /* When we enter this execution branch, assert() will always
+ * fail. This one just provides more informative message than
+ * simple assert(FAIL); */
+ assert(nation-player == NULL);
+
+ /* Try to handle error situation as well as we can */
+ if (nation-player-nation == nation) {
+/* At least assignment is consistent. Leave nation assigned,
+ * and make sure that nation is also marked available. */
+nation-is_available = TRUE;
+ } else {
+/* Not consistent. Just initialize the pointer and hope for the best */
+nation-player = NULL;
+ }
+}
} nations_iterate_end;
send_ruleset_nations(game.est_connections);
}
@@ -1791,7 +1813,6 @@
con_flush();
- server_game_init();
stdinhand_init();
diplhand_init();
@@ -1801,6 +1822,8 @@
server_open_socket();
}
+ server_game_init();
+
/* load a saved game */
if (srvarg.load_filename[0] != '\0') {
(void) load_command(NULL, srvarg.load_filename, FALSE);
@@ -1889,8 +1912,6 @@
**/
static void srv_loop(void)
{
- init_available_nations();
-
freelog(LOG_NORMAL, _(Now accepting new client connections.));
while(server_state == PRE_GAME_STATE) {
sniff_packets(); /* Accepting commands. */
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39511) [Patch] Call init_available_nations()
URL: http://bugs.freeciv.org/Ticket/Display.html?id=39511
On 08/08/07, Marko Lindqvist [EMAIL PROTECTED] wrote:
There was another call of init_available_nations() at wrong time. And
sometimes it was not called when it should have been called.
- Fixed compiler warning
- ML
diff -Nurd -X.diff_ignore freeciv/common/player.c freeciv/common/player.c
--- freeciv/common/player.c 2007-08-04 18:38:32.0 +0300
+++ freeciv/common/player.c 2007-08-09 11:19:04.0 +0300
@@ -330,6 +330,9 @@
}
pplayer-nation = pnation;
return TRUE;
+ } else {
+/* Nation already assigned to this player */
+assert(pnation-player == pplayer);
}
return FALSE;
}
diff -Nurd -X.diff_ignore freeciv/server/ruleset.c freeciv/server/ruleset.c
--- freeciv/server/ruleset.c 2007-08-07 14:24:36.0 +0300
+++ freeciv/server/ruleset.c 2007-08-09 11:23:54.0 +0300
@@ -46,6 +46,7 @@
#include aiunit.h /* update_simple_ai_types */
#include ruleset.h
+#include srv_main.h
/* RULESET_SUFFIX already used, no leading dot here */
#define RULES_SUFFIX ruleset
@@ -3494,6 +3495,10 @@
static void reset_player_nations(void)
{
players_iterate(pplayer) {
+/* We cannot use player_set_nation() here since this is
+ * called before nations are loaded. Player pointers
+ * from nations will be initialized to NULL when nations are
+ * loaded. */
pplayer-nation = NO_NATION_SELECTED;
pplayer-city_style = 0;
} players_iterate_end;
@@ -3548,6 +3553,9 @@
load_ruleset_effects(effectfile);
load_ruleset_game();
+ /* Init nations we just loaded. */
+ init_available_nations();
+
sanity_check_ruleset_data();
precalc_tech_data();
diff -Nurd -X.diff_ignore freeciv/server/srv_main.c freeciv/server/srv_main.c
--- freeciv/server/srv_main.c 2007-08-07 00:07:45.0 +0300
+++ freeciv/server/srv_main.c 2007-08-09 11:19:04.0 +0300
@@ -1347,7 +1347,29 @@
}
}
nations_iterate(nation) {
-nation-player = NULL;
+/* Even though this function is called init_available_nations(),
+ * nation-player should never have value assigned to it
+ * (since it has beeen initialized in load_rulesets() ). */
+if (nation-player != NULL) {
+
+ freelog(LOG_ERROR, Player assigned to nation before
+ init_available_nations());
+
+ /* When we enter this execution branch, assert() will always
+ * fail. This one just provides more informative message than
+ * simple assert(FAIL); */
+ assert(nation-player == NULL);
+
+ /* Try to handle error situation as well as we can */
+ if (nation-player-nation == nation) {
+/* At least assignment is consistent. Leave nation assigned,
+ * and make sure that nation is also marked available. */
+nation-is_available = TRUE;
+ } else {
+/* Not consistent. Just initialize the pointer and hope for the best */
+nation-player = NULL;
+ }
+}
} nations_iterate_end;
send_ruleset_nations(game.est_connections);
}
@@ -1889,8 +1911,6 @@
**/
static void srv_loop(void)
{
- init_available_nations();
-
freelog(LOG_NORMAL, _(Now accepting new client connections.));
while(server_state == PRE_GAME_STATE) {
sniff_packets(); /* Accepting commands. */
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39511) [Patch] Call init_available_nations()
URL: http://bugs.freeciv.org/Ticket/Display.html?id=39511
On 09/08/07, Marko Lindqvist [EMAIL PROTECTED] wrote:
On 08/08/07, Marko Lindqvist [EMAIL PROTECTED] wrote:
There was another call of init_available_nations() at wrong time. And
sometimes it was not called when it should have been called.
- Fixed compiler warning
- Fixed crash caused by uninitialized connection list
- ML
diff -Nurd -X.diff_ignore freeciv/common/player.c freeciv/common/player.c
--- freeciv/common/player.c 2007-08-04 18:38:32.0 +0300
+++ freeciv/common/player.c 2007-08-09 11:38:27.0 +0300
@@ -330,6 +330,9 @@
}
pplayer-nation = pnation;
return TRUE;
+ } else {
+/* Nation already assigned to this player */
+assert(pnation-player == pplayer);
}
return FALSE;
}
diff -Nurd -X.diff_ignore freeciv/server/ruleset.c freeciv/server/ruleset.c
--- freeciv/server/ruleset.c 2007-08-07 14:24:36.0 +0300
+++ freeciv/server/ruleset.c 2007-08-09 11:38:27.0 +0300
@@ -46,6 +46,7 @@
#include aiunit.h /* update_simple_ai_types */
#include ruleset.h
+#include srv_main.h
/* RULESET_SUFFIX already used, no leading dot here */
#define RULES_SUFFIX ruleset
@@ -3494,6 +3495,10 @@
static void reset_player_nations(void)
{
players_iterate(pplayer) {
+/* We cannot use player_set_nation() here since this is
+ * called before nations are loaded. Player pointers
+ * from nations will be initialized to NULL when nations are
+ * loaded. */
pplayer-nation = NO_NATION_SELECTED;
pplayer-city_style = 0;
} players_iterate_end;
@@ -3548,6 +3553,9 @@
load_ruleset_effects(effectfile);
load_ruleset_game();
+ /* Init nations we just loaded. */
+ init_available_nations();
+
sanity_check_ruleset_data();
precalc_tech_data();
diff -Nurd -X.diff_ignore freeciv/server/srv_main.c freeciv/server/srv_main.c
--- freeciv/server/srv_main.c 2007-08-07 00:07:45.0 +0300
+++ freeciv/server/srv_main.c 2007-08-09 11:44:53.0 +0300
@@ -1347,7 +1347,29 @@
}
}
nations_iterate(nation) {
-nation-player = NULL;
+/* Even though this function is called init_available_nations(),
+ * nation-player should never have value assigned to it
+ * (since it has beeen initialized in load_rulesets() ). */
+if (nation-player != NULL) {
+
+ freelog(LOG_ERROR, Player assigned to nation before
+ init_available_nations());
+
+ /* When we enter this execution branch, assert() will always
+ * fail. This one just provides more informative message than
+ * simple assert(FAIL); */
+ assert(nation-player == NULL);
+
+ /* Try to handle error situation as well as we can */
+ if (nation-player-nation == nation) {
+/* At least assignment is consistent. Leave nation assigned,
+ * and make sure that nation is also marked available. */
+nation-is_available = TRUE;
+ } else {
+/* Not consistent. Just initialize the pointer and hope for the best */
+nation-player = NULL;
+ }
+}
} nations_iterate_end;
send_ruleset_nations(game.est_connections);
}
@@ -1791,7 +1813,6 @@
con_flush();
- server_game_init();
stdinhand_init();
diplhand_init();
@@ -1801,6 +1822,8 @@
server_open_socket();
}
+ server_game_init();
+
/* load a saved game */
if (srvarg.load_filename[0] != '\0') {
(void) load_command(NULL, srvarg.load_filename, FALSE);
@@ -1889,8 +1912,6 @@
**/
static void srv_loop(void)
{
- init_available_nations();
-
freelog(LOG_NORMAL, _(Now accepting new client connections.));
while(server_state == PRE_GAME_STATE) {
sniff_packets(); /* Accepting commands. */
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#39511) [Patch] Call init_available_nations()
URL: http://bugs.freeciv.org/Ticket/Display.html?id=39511
There was another call of init_available_nations() at wrong time. And
sometimes it was not called when it should have been called.
Attached patch moves init_available_nations() call to where it really
belong. Always initialize nations when their structure is created.
Never initialize them after they have been assigned to players.
In addition to moving that one call, added several sanity checks and comments.
- ML
diff -Nurd -X.diff_ignore freeciv/common/player.c freeciv/common/player.c
--- freeciv/common/player.c 2007-08-04 18:38:32.0 +0300
+++ freeciv/common/player.c 2007-08-08 17:54:00.0 +0300
@@ -330,6 +330,9 @@
}
pplayer-nation = pnation;
return TRUE;
+ } else {
+/* Nation already assigned to this player */
+assert(pnation-player == pplayer);
}
return FALSE;
}
diff -Nurd -X.diff_ignore freeciv/server/ruleset.c freeciv/server/ruleset.c
--- freeciv/server/ruleset.c 2007-08-07 14:24:36.0 +0300
+++ freeciv/server/ruleset.c 2007-08-08 17:46:38.0 +0300
@@ -3494,6 +3494,10 @@
static void reset_player_nations(void)
{
players_iterate(pplayer) {
+/* We cannot use player_set_nation() here since this is
+ * called before nations are loaded. Player pointers
+ * from nations will be initialized to NULL when nations are
+ * loaded. */
pplayer-nation = NO_NATION_SELECTED;
pplayer-city_style = 0;
} players_iterate_end;
@@ -3548,6 +3552,9 @@
load_ruleset_effects(effectfile);
load_ruleset_game();
+ /* Init nations we just loaded. */
+ init_available_nations();
+
sanity_check_ruleset_data();
precalc_tech_data();
diff -Nurd -X.diff_ignore freeciv/server/srv_main.c freeciv/server/srv_main.c
--- freeciv/server/srv_main.c 2007-08-07 00:07:45.0 +0300
+++ freeciv/server/srv_main.c 2007-08-08 18:00:16.0 +0300
@@ -1347,7 +1347,29 @@
}
}
nations_iterate(nation) {
-nation-player = NULL;
+/* Even though this function is called init_available_nations(),
+ * nation-player should never have value assigned to it
+ * (since it has beeen initialized in load_rulesets() ). */
+if (nation-player != NULL) {
+
+ freelog(LOG_ERROR, Player assigned to nation before
+ init_available_nations());
+
+ /* When we enter this execution branch, assert() will always
+ * fail. This one just provides more informative message than
+ * simple assert(FAIL); */
+ assert(nation-player == NULL);
+
+ /* Try to handle error situation as well as we can */
+ if (nation-player-nation == nation) {
+/* At least assignment is consistent. Leave nation assigned,
+ * and make sure that nation is also marked available. */
+nation-is_available = TRUE;
+ } else {
+/* Not consistent. Just initialize the pointer and hope for the best */
+nation-player = NULL;
+ }
+}
} nations_iterate_end;
send_ruleset_nations(game.est_connections);
}
@@ -1889,8 +1911,6 @@
**/
static void srv_loop(void)
{
- init_available_nations();
-
freelog(LOG_NORMAL, _(Now accepting new client connections.));
while(server_state == PRE_GAME_STATE) {
sniff_packets(); /* Accepting commands. */
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
