[Freeciv-Dev] Re: (PR#10459) bug: pplayer_can_ally assert fail

["Marko Lindqvist" <cazfi74@xxxxxxxxx>]

<URL: http://bugs.freeciv.org/Ticket/Display.html?id=10459 >

Jason Short wrote:
> 
>> [per - Mon Oct 18 18:14:02 2004]:
> 
>> Maybe we should just insert assert(pplayer->is_alive &&
>> pplayer2->is_alive) in these functions, and catch all erronous callers?
> 
> Will someone make a patch?

  Here.

  Asserts & corrections for development version, only corrections for 
stable branches.
  Maybe we should put only assertless version to dev too at first (while 
we fix most typical assert failures). Assert failed almost immediately.


  - ML

diff -Nurd -X.diff_ignore freeciv/common/player.c freeciv/common/player.c
--- freeciv/common/player.c     2006-07-08 23:33:28.656250000 +0300
+++ freeciv/common/player.c     2006-07-09 00:46:33.156250000 +0300
@@ -687,6 +687,8 @@
 bool pplayers_at_war(const struct player *pplayer,
                      const struct player *pplayer2)
 {
+  assert(pplayer->is_alive && pplayer2->is_alive);
+
   enum diplstate_type ds = pplayer_get_diplstate(pplayer, pplayer2)->type;
   if (pplayer == pplayer2) {
     return FALSE;
@@ -703,6 +705,8 @@
 bool pplayers_allied(const struct player *pplayer,
                      const struct player *pplayer2)
 {
+  assert(pplayer->is_alive && pplayer2->is_alive);
+
   enum diplstate_type ds = pplayer_get_diplstate(pplayer, pplayer2)->type;
   if (pplayer == pplayer2) {
     return TRUE;
@@ -719,6 +723,8 @@
 bool pplayers_in_peace(const struct player *pplayer,
                        const struct player *pplayer2)
 {
+  assert(pplayer->is_alive && pplayer2->is_alive);
+
   enum diplstate_type ds = pplayer_get_diplstate(pplayer, pplayer2)->type;
 
   if (pplayer == pplayer2) {
@@ -736,6 +742,8 @@
 bool pplayers_non_attack(const struct player *pplayer,
                          const struct player *pplayer2)
 {
+  assert(pplayer->is_alive && pplayer2->is_alive);
+
   enum diplstate_type ds = pplayer_get_diplstate(pplayer, pplayer2)->type;
   if (pplayer == pplayer2) {
     return FALSE;
diff -Nurd -X.diff_ignore freeciv/server/sanitycheck.c 
freeciv/server/sanitycheck.c
--- freeciv/server/sanitycheck.c        2006-07-08 23:35:33.953125000 +0300
+++ freeciv/server/sanitycheck.c        2006-07-09 00:48:39.390625000 +0300
@@ -463,9 +463,9 @@
        SANITY_CHECK(pplayer->diplstates[pplayer2->player_no].turns_left
               == pplayer2->diplstates[pplayer->player_no].turns_left);
       }
-      if (pplayers_allied(pplayer, pplayer2)
-          && pplayer->is_alive
-          && pplayer2->is_alive) {
+      if (pplayer->is_alive
+          && pplayer2->is_alive
+          && pplayers_allied(pplayer, pplayer2)) {
         SANITY_CHECK(pplayer_can_make_treaty(pplayer, pplayer2, DS_ALLIANCE)
                      != DIPL_ALLIANCE_PROBLEM);
       }
diff -Nurd -X.diff_ignore freeciv/server/savegame.c freeciv/server/savegame.c
--- freeciv/server/savegame.c   2006-07-08 23:35:34.000000000 +0300
+++ freeciv/server/savegame.c   2006-07-09 00:46:33.250000000 +0300
@@ -3855,7 +3855,8 @@
     /* Sanity check alliances, prevent allied-with-ally-of-enemy */
     players_iterate(plr) {
       players_iterate(aplayer) {
-        if (pplayers_allied(plr, aplayer)
+        if (plr->is_alive && aplayer->is_alive
+            && pplayers_allied(plr, aplayer)
             && pplayer_can_make_treaty(plr, aplayer, DS_ALLIANCE) 
                == DIPL_ALLIANCE_PROBLEM) {
           freelog(LOG_ERROR, _("Illegal alliance structure detected: "

diff -Nurd -X.diff_ignore freeciv/server/sanitycheck.c 
freeciv/server/sanitycheck.c
--- freeciv/server/sanitycheck.c        2006-07-09 00:58:26.687500000 +0300
+++ freeciv/server/sanitycheck.c        2006-07-09 00:58:48.468750000 +0300
@@ -463,9 +463,9 @@
        SANITY_CHECK(pplayer->diplstates[pplayer2->player_no].turns_left
               == pplayer2->diplstates[pplayer->player_no].turns_left);
       }
-      if (pplayers_allied(pplayer, pplayer2)
-          && pplayer->is_alive
-          && pplayer2->is_alive) {
+      if (pplayer->is_alive
+          && pplayer2->is_alive
+          && pplayers_allied(pplayer, pplayer2)) {
         SANITY_CHECK(pplayer_can_make_treaty(pplayer, pplayer2, DS_ALLIANCE)
                      != DIPL_ALLIANCE_PROBLEM);
       }
diff -Nurd -X.diff_ignore freeciv/server/savegame.c freeciv/server/savegame.c
--- freeciv/server/savegame.c   2006-07-09 00:58:26.687500000 +0300
+++ freeciv/server/savegame.c   2006-07-09 00:16:17.562500000 +0300
@@ -3862,7 +3862,9 @@
     /* Sanity check alliances, prevent allied-with-ally-of-enemy */
     players_iterate(plr) {
       players_iterate(aplayer) {
-        if (pplayers_allied(plr, aplayer)
+        if (plr->is_alive
+            && aplayer->is_alive
+            && pplayers_allied(plr, aplayer)
             && pplayer_can_make_treaty(plr, aplayer, DS_ALLIANCE) 
                == DIPL_ALLIANCE_PROBLEM) {
           freelog(LOG_ERROR, _("Illegal alliance structure detected: "

diff -Nurd -X.diff_ignore freeciv/server/savegame.c freeciv/server/savegame.c
--- freeciv/server/savegame.c   2006-07-09 00:26:17.781250000 +0300
+++ freeciv/server/savegame.c   2006-07-09 00:56:29.843750000 +0300
@@ -1896,7 +1896,9 @@
   }
   /* Sanity check alliances, prevent allied-with-ally-of-enemy */
   players_iterate(aplayer) {
-    if (pplayers_allied(plr, aplayer)
+    if (plr->is_alive
+        && aplayer->is_alive
+        && pplayers_allied(plr, aplayer)
         && !pplayer_can_ally(plr, aplayer)) {
       freelog(LOG_ERROR, _("Illegal alliance structure detected: "
               "%s's alliance to %s reduced to peace treaty."),