<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
