Author: sveinung
Date: Fri Oct 23 13:40:15 2015
New Revision: 30183

URL: http://svn.gna.org/viewcvs/freeciv?rev=30183&view=rev
Log:
Don't get the id from a potentially dead struct

Don't access a potentially dead city or unit to get its id number to check
if it is alive. There is no way to know what that memory contains if it is
dead.

Use the already existing actor_id and target_id when unit_perform_action()
checks that actor and target survived the Lua.

The checks in the action doer functions are redundant because of the checks
in unit_perform_action(). Drop them.

Reported by Marko Lindqvist <cazfi>

See bug #23956

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=30183&r1=30182&r2=30183&view=diff
==============================================================================
--- branches/S2_6/server/diplomats.c    (original)
+++ branches/S2_6/server/diplomats.c    Fri Oct 23 13:40:15 2015
@@ -95,7 +95,7 @@
   }
 
   /* Sanity check: The actor still exists. */
-  if (!pplayer || !pdiplomat || !unit_alive(pdiplomat->id)) {
+  if (!pplayer || !pdiplomat) {
     return;
   }
 
@@ -172,7 +172,7 @@
     return;
 
   /* Sanity check: The actor still exists. */
-  if (!pplayer || !pdiplomat || !unit_alive(pdiplomat->id)) {
+  if (!pplayer || !pdiplomat) {
     return;
   }
 
@@ -276,7 +276,7 @@
   }
 
   /* Sanity check: The actor still exists. */
-  if (!pplayer || !pdiplomat || !unit_alive(pdiplomat->id)) {
+  if (!pplayer || !pdiplomat) {
     return;
   }
 
@@ -332,7 +332,7 @@
   struct player *uplayer;
 
   /* Fetch target unit's player.  Sanity checks. */
-  if (!pvictim || !unit_alive(pvictim->id)) {
+  if (!pvictim) {
     return;
   }
 
@@ -342,7 +342,7 @@
   }
 
   /* Sanity check: The actor still exists. */
-  if (!pplayer || !pdiplomat || !unit_alive(pdiplomat->id)) {
+  if (!pplayer || !pdiplomat) {
     return;
   }
 
@@ -427,7 +427,7 @@
   struct city *pcity;
 
   /* Fetch target unit's player.  Sanity checks. */
-  if (!pvictim || !unit_alive(pvictim->id)) {
+  if (!pvictim) {
     return;
   }
   uplayer = unit_owner(pvictim);
@@ -436,7 +436,7 @@
   }
 
   /* Sanity check: The actor still exists. */
-  if (!pplayer || !pdiplomat || !unit_alive(pdiplomat->id)) {
+  if (!pplayer || !pdiplomat) {
     return;
   }
 
@@ -584,7 +584,7 @@
   }
 
   /* Sanity check: The actor still exists. */
-  if (!pplayer || !pdiplomat || !unit_alive(pdiplomat->id)) {
+  if (!pplayer || !pdiplomat) {
     return;
   }
 
@@ -746,7 +746,7 @@
   }
 
   /* Sanity check: The actor still exists. */
-  if (!pplayer || !pdiplomat || !unit_alive(pdiplomat->id)) {
+  if (!pplayer || !pdiplomat) {
     return;
   }
 
@@ -879,7 +879,7 @@
   }
 
   /* Sanity check: The actor still exists. */
-  if (!pplayer || !pdiplomat || !unit_alive(pdiplomat->id)) {
+  if (!pplayer || !pdiplomat) {
     return;
   }
 
@@ -1128,7 +1128,7 @@
   int gold_give;
 
   /* Sanity check: The actor still exists. */
-  if (!act_player || !act_unit || !unit_alive(act_unit->id)) {
+  if (!act_player || !act_unit) {
     return;
   }
 

Modified: branches/S2_6/server/unithand.c
URL: 
http://svn.gna.org/viewcvs/freeciv/branches/S2_6/server/unithand.c?rev=30183&r1=30182&r2=30183&view=diff
==============================================================================
--- branches/S2_6/server/unithand.c     (original)
+++ branches/S2_6/server/unithand.c     Fri Oct 23 13:40:15 2015
@@ -252,11 +252,6 @@
   char capturer_link[MAX_LEN_LINK];
   const char *capturer_nation = nation_plural_for_player(pplayer);
   bv_unit_types unique_on_tile;
-
-  /* Sanity check: The actor is still alive. */
-  if (!unit_alive(punit->id)) {
-    return;
-  }
 
   /* Sanity check: make sure that the capture won't result in the actor
    * ending up with more than one unit of each unique unit type. */
@@ -1063,11 +1058,11 @@
                             API_TYPE_ACTION, action_by_number(action),    \
                             API_TYPE_UNIT, actor,                         \
                             API_TYPE_CITY, target);                       \
-  if (!actor || !unit_alive(actor->id)) {                                 \
+  if (!actor || !unit_alive(actor_id)) {                                  \
     /* Actor unit was destroyed during pre action Lua. */                 \
     return;                                                         \
   }                                                                       \
-  if (!target || !city_exist(target->id)) {                               \
+  if (!target || !city_exist(target_id)) {                                \
     /* Target city was destroyed during pre action Lua. */                \
     return;                                                         \
   }
@@ -1077,11 +1072,11 @@
                             API_TYPE_ACTION, action_by_number(action),    \
                             API_TYPE_UNIT, actor,                         \
                             API_TYPE_UNIT, target);                       \
-  if (!actor || !unit_alive(actor->id)) {                                 \
+  if (!actor || !unit_alive(actor_id)) {                                  \
     /* Actor unit was destroyed during pre action Lua. */                 \
     return;                                                         \
   }                                                                       \
-  if (!target || !unit_alive(target->id)) {                               \
+  if (!target || !unit_alive(target_id)) {                                \
     /* Target unit was destroyed during pre action Lua. */                \
     return;                                                         \
   }
@@ -2457,11 +2452,6 @@
     return;
   }
 
-  /* Sanity check: The actor is still alive. */
-  if (!unit_alive(punit->id)) {
-    return;
-  }
-
   /* Sanity check: The target city still exists. */
   if (NULL == pcity_dest) {
     return;
@@ -2551,11 +2541,6 @@
 
   if (NULL == punit) {
     /* Probably died or bribed. */
-    return FALSE;
-  }
-
-  /* Sanity check: The actor is still alive. */
-  if (!unit_alive(punit->id)) {
     return FALSE;
   }
 


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

Reply via email to