<URL: http://bugs.freeciv.org/Ticket/Display.html?id=39842 >
On Wed, 7 Nov 2007 12:56:25 -0800 Egor Vyscrebentsov wrote: > Good daytime! > > branches/s2_1, r13919 > > Start a game, say 'read civ2.serv' while game is already started and see > gui-gtk segfault... Say 'quit' and see segfault of civserver... Not gtk2-specific, but common client problem, of course. > Reason: nations are already freed when nation_of_player() is called. > And both nation_of_player() and bounds_check_nation() don't check for > nation is NULL. Attached patch makes civ{server,client} at least show an error rather than segfaulting. > Questions: > 1. Why 'rulesetdir' isn't disallowed in already started game? Still need an answer. Since several settings with similar effect are disallowed during the game, this command should be disallowed too. And I think this is a core of current bug, but it will be nice if our functions will work even with wrong parameters at input. > 2. How to fix usage of nation_of_player()? [ie, what way is better?] More hard question. Now this function call die() if bounds check is not passed. I'm not sure that this is right reaction in all ways. Maybe there could be a parameter, say 'allow_null_nation', that will has default value FALSE? Or we are sure that there is no and will be no variants when nation may absent? (I don't know yet how this work with observers.) Any thoughts? > 3. Is there a reason not to have a check in bounds_check_nation()? Ok, no reason, I guess. Added in patch, returns FALSE. PS. Message "This setting can't be modified after the game has started." looks badly than "Setting '%s' can't be modified ..." when loading .serv file (since command doesn't shown). -- Thanks, evyscr
Index: common/nation.c =================================================================== --- common/nation.c (revision 13921) +++ common/nation.c (working copy) @@ -49,6 +49,10 @@ freelog(loglevel, "%s before nations setup", func_name); return FALSE; } + if (!pnation) { + freelog(loglevel, "%s error: nation is NULL", func_name); + return FALSE; + } if (pnation->index < 0 || pnation->index >= game.control.nation_count || &nations[pnation->index] != pnation) { @@ -252,7 +256,11 @@ { assert(plr != NULL); if (!bounds_check_nation(plr->nation, LOG_FATAL, "nation_of_player")) { - die("wrong nation %d", plr->nation->index); + if (plr->nation) { + die("wrong nation %d", plr->nation->index); + } else { + die("wrong nation: null nation"); + } } return plr->nation; }
_______________________________________________ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev