Author: sveinung
Date: Sat Oct 24 23:37:38 2015
New Revision: 30203

URL: http://svn.gna.org/viewcvs/freeciv?rev=30203&view=rev
Log:
Make target/actor exists sanity checks asserts

The checks that actor and target survived the Lua in the action doer
functions are redundant because of the checks in unit_perform_action().
Change them into asserts.

See patch #6467

Modified:
    branches/S2_6/server/diplomats.c
    branches/S2_6/server/unithand.c

Modified: branches/S2_6/server/diplomats.c
URL: 
http://svn.gna.org/viewcvs/freeciv/branches/S2_6/server/diplomats.c?rev=30203&r1=30202&r2=30203&view=diff
==============================================================================
--- branches/S2_6/server/diplomats.c    (original)
+++ branches/S2_6/server/diplomats.c    Sat Oct 24 23:37:38 2015
@@ -85,19 +85,13 @@
   const char *clink;
 
   /* Fetch target city's player.  Sanity checks. */
-  if (!pcity) {
-    return;
-  }
-
+  fc_assert_ret(pcity);
   cplayer = city_owner(pcity);
-  if (!cplayer) {
-    return;
-  }
+  fc_assert_ret(cplayer);
 
   /* Sanity check: The actor still exists. */
-  if (!pplayer || !pdiplomat) {
-    return;
-  }
+  fc_assert_ret(pplayer);
+  fc_assert_ret(pdiplomat);
 
   ctile = city_tile(pcity);
   clink = city_link(pcity);
@@ -163,16 +157,17 @@
   struct packet_city_info city_packet;
 
   /* Fetch target city's player.  Sanity checks. */
-  if (!pcity) {
-    return;
-  }
-
-  cplayer = city_owner (pcity);
-  if ((cplayer == pplayer) || !cplayer)
-    return;
+  fc_assert_ret(pcity);
+  cplayer = city_owner(pcity);
+  fc_assert_ret(cplayer);
 
   /* Sanity check: The actor still exists. */
-  if (!pplayer || !pdiplomat) {
+  fc_assert_ret(pplayer);
+  fc_assert_ret(pdiplomat);
+
+  /* Sanity check: The target is foreign. */
+  if (cplayer == pplayer) {
+    /* Nothing to do to a domestic target. */
     return;
   }
 
@@ -266,17 +261,17 @@
   struct player *cplayer;
 
   /* Fetch target city's player.  Sanity checks. */
-  if (!pcity) {
-    return;
-  }
-
+  fc_assert_ret(pcity);
   cplayer = city_owner(pcity);
-  if ((cplayer == pplayer) || !cplayer) {
-    return;
-  }
+  fc_assert_ret(cplayer);
 
   /* Sanity check: The actor still exists. */
-  if (!pplayer || !pdiplomat) {
+  fc_assert_ret(pplayer);
+  fc_assert_ret(pdiplomat);
+
+  /* Sanity check: The target is foreign. */
+  if (cplayer == pplayer) {
+    /* Nothing to do to a domestic target. */
     return;
   }
 
@@ -332,19 +327,13 @@
   struct player *uplayer;
 
   /* Fetch target unit's player.  Sanity checks. */
-  if (!pvictim) {
-    return;
-  }
-
+  fc_assert_ret(pvictim);
   uplayer = unit_owner(pvictim);
-  if (!uplayer) {
-    return;
-  }
+  fc_assert_ret(uplayer);
 
   /* Sanity check: The actor still exists. */
-  if (!pplayer || !pdiplomat) {
-    return;
-  }
+  fc_assert_ret(pplayer);
+  fc_assert_ret(pdiplomat);
 
   log_debug("sabotage-unit: unit: %d", pdiplomat->id);
 
@@ -427,18 +416,13 @@
   struct city *pcity;
 
   /* Fetch target unit's player.  Sanity checks. */
-  if (!pvictim) {
-    return;
-  }
+  fc_assert_ret(pvictim);
   uplayer = unit_owner(pvictim);
-  if (!uplayer) {
-    return;
-  }
+  fc_assert_ret(uplayer);
 
   /* Sanity check: The actor still exists. */
-  if (!pplayer || !pdiplomat) {
-    return;
-  }
+  fc_assert_ret(pplayer);
+  fc_assert_ret(pdiplomat);
 
   /* Sanity check: The target is foreign. */
   if (uplayer == pplayer) {
@@ -574,17 +558,17 @@
 
   /* We have to check arguments. They are sent to us by a client,
      so we cannot trust them */
-  if (!pcity) {
-    return;
-  }
-  
-  cplayer = city_owner (pcity);
-  if ((cplayer == pplayer) || !cplayer) {
-    return;
-  }
+  fc_assert_ret(pcity);
+  cplayer = city_owner(pcity);
+  fc_assert_ret(cplayer);
 
   /* Sanity check: The actor still exists. */
-  if (!pplayer || !pdiplomat) {
+  fc_assert_ret(pplayer);
+  fc_assert_ret(pdiplomat);
+
+  /* Sanity check: The target is foreign. */
+  if (cplayer == pplayer) {
+    /* Nothing to do to a domestic target. */
     return;
   }
 
@@ -736,19 +720,13 @@
   int revolt_cost;
 
   /* Fetch target civilization's player.  Sanity checks. */
-  if (!pcity) {
-    return;
-  }
-
-  cplayer = city_owner (pcity);
-  if (!cplayer) {
-    return;
-  }
+  fc_assert_ret(pcity);
+  cplayer = city_owner(pcity);
+  fc_assert_ret(cplayer);
 
   /* Sanity check: The actor still exists. */
-  if (!pplayer || !pdiplomat) {
-    return;
-  }
+  fc_assert_ret(pplayer);
+  fc_assert_ret(pdiplomat);
 
   /* Sanity check: The target is foreign. */
   if (cplayer == pplayer) {
@@ -869,19 +847,13 @@
   int success_prob;
 
   /* Fetch target city's player.  Sanity checks. */
-  if (!pcity) {
-    return;
-  }
-
-  cplayer = city_owner (pcity);
-  if (!cplayer) {
-    return;
-  }
+  fc_assert_ret(pcity);
+  cplayer = city_owner(pcity);
+  fc_assert_ret(cplayer);
 
   /* Sanity check: The actor still exists. */
-  if (!pplayer || !pdiplomat) {
-    return;
-  }
+  fc_assert_ret(pplayer);
+  fc_assert_ret(pdiplomat);
 
   log_debug("sabotage: unit: %d", pdiplomat->id);
 
@@ -1128,22 +1100,17 @@
   int gold_give;
 
   /* Sanity check: The actor still exists. */
-  if (!act_player || !act_unit) {
-    return;
-  }
+  fc_assert_ret(act_player);
+  fc_assert_ret(act_unit);
 
   /* Sanity check: The target city still exists. */
-  if (!tgt_city) {
-    return;
-  }
+  fc_assert_ret(tgt_city);
 
   /* Who to steal from. */
   tgt_player = city_owner(tgt_city);
 
   /* Sanity check: The target player still exists. */
-  if (!tgt_player) {
-    return;
-  }
+  fc_assert_ret(tgt_player);
 
   /* Sanity check: The target is foreign. */
   if (tgt_player == act_player) {

Modified: branches/S2_6/server/unithand.c
URL: 
http://svn.gna.org/viewcvs/freeciv/branches/S2_6/server/unithand.c?rev=30203&r1=30202&r2=30203&view=diff
==============================================================================
--- branches/S2_6/server/unithand.c     (original)
+++ branches/S2_6/server/unithand.c     Sat Oct 24 23:37:38 2015
@@ -2447,15 +2447,12 @@
 {
   const char *work;
 
-  if (NULL == punit) {
-    /* Probably died or bribed. */
-    return;
-  }
+  /* Sanity check: The actor still exists. */
+  fc_assert_ret(pplayer);
+  fc_assert_ret(punit);
 
   /* Sanity check: The target city still exists. */
-  if (NULL == pcity_dest) {
-    return;
-  }
+  fc_assert_ret(pcity_dest);
 
   pcity_dest->shield_stock += unit_build_shield_cost(punit);
   pcity_dest->caravan_shields += unit_build_shield_cost(punit);
@@ -2539,14 +2536,12 @@
   enum traderoute_bonus_type bonus_type;
   const char *bonus_str;
 
-  if (NULL == punit) {
-    /* Probably died or bribed. */
-    return FALSE;
-  }
-
-  if (!pcity_dest) {
-    return FALSE;
-  }
+  /* Sanity check: The actor still exists. */
+  fc_assert_ret_val(pplayer, FALSE);
+  fc_assert_ret_val(punit, FALSE);
+
+  /* Sanity check: The target city still exists. */
+  fc_assert_ret_val(pcity_dest, FALSE);
 
   pcity_homecity = player_city_by_number(pplayer, punit->homecity);
 


_______________________________________________
Freeciv-commits mailing list
Freeciv-commits@gna.org
https://mail.gna.org/listinfo/freeciv-commits

Reply via email to