Re: [Freeciv-Dev] (PR#38558) [patch] client segfault during automove with 30 players and 2 pirates
URL: http://bugs.freeciv.org/Ticket/Display.html?id=38558 On 12/08/2007, Marko Lindqvist wrote: Attached related patches for S2_1 and trunk. S2_1 just adds client side legality check for phase number. Trunk version adds several asserts to help finding root cause bug. As bug itself is (probably) already fixed, I'm going to commit version without additional asserts to trunk (and S2_2) also. - ML ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#38558) [patch] client segfault during automove with 30 players and 2 pirates
URL: http://bugs.freeciv.org/Ticket/Display.html?id=38558 On 12/08/07, Marko Lindqvist [EMAIL PROTECTED] wrote: On 21/03/07, [EMAIL PROTECTED] [EMAIL PROTECTED] wrote: If the player list is full (all player and pirate slots full) the client may crash during automove phase. This is caused by the server reporting phase 32 (which is invalid). So far I have been unable to reproduce this. Found no errors by looking server sources either. #39392 could explain this. Making packet validity checks is still not bad idea - patches sent earlier should go in. - ML ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#38558) [patch] client segfault during automove with 30 players and 2 pirates
URL: http://bugs.freeciv.org/Ticket/Display.html?id=38558 On 21/03/07, [EMAIL PROTECTED] [EMAIL PROTECTED] wrote: If the player list is full (all player and pirate slots full) the client may crash during automove phase. This is caused by the server reporting phase 32 (which is invalid). So far I have been unable to reproduce this. Found no errors by looking server sources either. Can you give me more detailed instructions on how to reproduce. If you have savegame from which this can be reproduced, please send it to me. Of course we should add client end sanity check for phase number, but core bug here is that server is sending illegal phase number in the first place. - ML ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#38558) [patch] client segfault during automove with 30 players and 2 pirates
URL: http://bugs.freeciv.org/Ticket/Display.html?id=38558
On 12/08/07, Marko Lindqvist [EMAIL PROTECTED] wrote:
So far I have been unable to reproduce this. Found no errors by
looking server sources either.
Can you give me more detailed instructions on how to reproduce. If
you have savegame from which this can be reproduced, please send it to
me.
Attached related patches for S2_1 and trunk. S2_1 just adds client
side legality check for phase number. Trunk version adds several
asserts to help finding root cause bug.
These do not remove need to find root cause bug.
- ML
diff -Nurd -X.diff_ignore freeciv/client/packhand.c freeciv/client/packhand.c
--- freeciv/client/packhand.c 2007-08-06 18:09:20.0 +0300
+++ freeciv/client/packhand.c 2007-08-07 11:47:08.0 +0300
@@ -861,6 +861,14 @@
**/
void handle_start_phase(int phase)
{
+ assert(phase = 0 phase game.info.nplayers);
+
+ if (phase 0 || phase = game.info.nplayers) {
+/* Illegal phase */
+freelog(LOG_ERROR, Illegal phase %d received from server!, phase);
+return;
+ }
+
game.info.phase = phase;
if (game.player_ptr is_player_phase(game.player_ptr, phase)) {
@@ -1382,6 +1390,7 @@
bool boot_help;
bool update_aifill_button = FALSE;
+ assert(game.info.nplayers = MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS);
if (game.info.aifill != pinfo-aifill) {
update_aifill_button = TRUE;
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-07 11:47:44.0 +0300
@@ -557,6 +557,7 @@
if (game.info.simultaneous_phases) {
game.info.num_phases = 1;
} else {
+assert(game.info.nplayers = MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS);
game.info.num_phases = game.info.nplayers;
}
send_game_info(NULL);
diff -Nurd -X.diff_ignore freeciv/client/packhand.c freeciv/client/packhand.c
--- freeciv/client/packhand.c 2007-08-11 12:17:15.0 +0300
+++ freeciv/client/packhand.c 2007-08-12 12:46:16.0 +0300
@@ -881,6 +881,13 @@
**/
void handle_start_phase(int phase)
{
+
+ if (phase 0 || phase = game.info.nplayers) {
+/* Illegal phase */
+freelog(LOG_ERROR, Illegal phase %d received from server!, phase);
+return;
+ }
+
game.info.phase = phase;
if (game.player_ptr is_player_phase(game.player_ptr, phase)) {
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#38558) [patch] client segfault during automove with 30 players and 2 pirates
URL: http://bugs.freeciv.org/Ticket/Display.html?id=38558
If the player list is full (all player and pirate slots full) the client may
crash during automove phase. This is caused by the server reporting phase 32
(which is invalid).
client/text.c:581 then calls
astr_add_line(str, _(Moving: %s), get_player(game.info.phase)-name);
as game.info.phase is out of range, get_player returns NULL, resulting in a
random pointer being passed to astr_add_line.
to correct this, this call should be changed to
if (is_valid_player_id(game.info.phase)) {
astr_add_line(str, _(Moving: %s), get_player(game.info.phase)-name);
}
___
Join Excite! - http://www.excite.com
The most personalized portal on the Web!
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
