Author: sveinung
Date: Fri Oct 23 13:39:13 2015
New Revision: 30179
URL: http://svn.gna.org/viewcvs/freeciv?rev=30179&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.
Drop the check in do_unit_disband(). It is called by handle_unit_disband()
and unit_do_disband_trad(). handle_unit_disband() gets its unit by looking
it up by id. unit_do_disband_trad() will return before the call if it
manages to disband the unit.
Reported by Marko Lindqvist <cazfi>
See bug #23956
Modified:
trunk/server/diplomats.c
trunk/server/unithand.c
Modified: trunk/server/diplomats.c
URL:
http://svn.gna.org/viewcvs/freeciv/trunk/server/diplomats.c?rev=30179&r1=30178&r2=30179&view=diff
==============================================================================
--- trunk/server/diplomats.c (original)
+++ trunk/server/diplomats.c Fri Oct 23 13:39:13 2015
@@ -105,7 +105,7 @@
}
/* Sanity check: The actor still exists. */
- if (!pplayer || !pdiplomat || !unit_alive(pdiplomat->id)) {
+ if (!pplayer || !pdiplomat) {
return FALSE;
}
@@ -188,7 +188,7 @@
return FALSE;
/* Sanity check: The actor still exists. */
- if (!pplayer || !pdiplomat || !unit_alive(pdiplomat->id)) {
+ if (!pplayer || !pdiplomat) {
return FALSE;
}
@@ -303,7 +303,7 @@
}
/* Sanity check: The actor still exists. */
- if (!pplayer || !pdiplomat || !unit_alive(pdiplomat->id)) {
+ if (!pplayer || !pdiplomat) {
return FALSE;
}
@@ -364,7 +364,7 @@
struct player *uplayer;
/* Fetch target unit's player. Sanity checks. */
- if (!pvictim || !unit_alive(pvictim->id)) {
+ if (!pvictim) {
return FALSE;
}
@@ -374,7 +374,7 @@
}
/* Sanity check: The actor still exists. */
- if (!pplayer || !pdiplomat || !unit_alive(pdiplomat->id)) {
+ if (!pplayer || !pdiplomat) {
return FALSE;
}
@@ -466,7 +466,7 @@
struct city *pcity;
/* Fetch target unit's player. Sanity checks. */
- if (!pvictim || !unit_alive(pvictim->id)) {
+ if (!pvictim) {
return FALSE;
}
uplayer = unit_owner(pvictim);
@@ -475,7 +475,7 @@
}
/* Sanity check: The actor still exists. */
- if (!pplayer || !pdiplomat || !unit_alive(pdiplomat->id)) {
+ if (!pplayer || !pdiplomat) {
return FALSE;
}
@@ -629,7 +629,7 @@
}
/* Sanity check: The actor still exists. */
- if (!pplayer || !pdiplomat || !unit_alive(pdiplomat->id)) {
+ if (!pplayer || !pdiplomat) {
return FALSE;
}
@@ -799,7 +799,7 @@
}
/* Sanity check: The actor still exists. */
- if (!pplayer || !pdiplomat || !unit_alive(pdiplomat->id)) {
+ if (!pplayer || !pdiplomat) {
return FALSE;
}
@@ -944,7 +944,7 @@
}
/* Sanity check: The actor still exists. */
- if (!pplayer || !pdiplomat || !unit_alive(pdiplomat->id)) {
+ if (!pplayer || !pdiplomat) {
return FALSE;
}
@@ -1207,7 +1207,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 FALSE;
}
@@ -1343,7 +1343,7 @@
const char *tgt_city_link;
/* Sanity check: The actor still exists. */
- if (!act_player || !act_unit || !unit_alive(act_unit->id)) {
+ if (!act_player || !act_unit) {
return FALSE;
}
@@ -1452,7 +1452,7 @@
const char *tgt_city_link;
/* Sanity check: The actor still exists. */
- if (!act_player || !act_unit || !unit_alive(act_unit->id)) {
+ if (!act_player || !act_unit) {
return FALSE;
}
Modified: trunk/server/unithand.c
URL:
http://svn.gna.org/viewcvs/freeciv/trunk/server/unithand.c?rev=30179&r1=30178&r2=30179&view=diff
==============================================================================
--- trunk/server/unithand.c (original)
+++ trunk/server/unithand.c Fri Oct 23 13:39:13 2015
@@ -287,11 +287,6 @@
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 FALSE;
- }
-
/* 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. */
BV_CLR_ALL(unique_on_tile);
@@ -402,7 +397,7 @@
/* Maybe it didn't survive the Lua call back. Why wasn't this caught by
* the caller? Check in the code that emits the signal. */
- fc_assert_ret_val(target && unit_alive(target->id), FALSE);
+ fc_assert_ret_val(target, FALSE);
uplayer = unit_owner(target);
@@ -411,7 +406,7 @@
/* Maybe it didn't survive the Lua call back. Why wasn't this caught by
* the caller? Check in the code that emits the signal. */
- fc_assert_ret_val(actor && unit_alive(actor->id), FALSE);
+ fc_assert_ret_val(actor, FALSE);
/* Where is the actor player? */
fc_assert_ret_val(pplayer, FALSE);
@@ -1488,11 +1483,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 FALSE; \
} \
- if (!target || !city_exist(target->id)) { \
+ if (!target || !city_exist(target_id)) { \
/* Target city was destroyed during pre action Lua. */ \
return FALSE; \
}
@@ -1501,7 +1496,7 @@
script_server_signal_emit("action_started_unit_self", 2, \
API_TYPE_ACTION, action_by_number(action), \
API_TYPE_UNIT, actor); \
- if (!actor || !unit_alive(actor->id)) { \
+ if (!actor || !unit_alive(actor_id)) { \
/* Actor unit was destroyed during pre action Lua. */ \
return FALSE; \
}
@@ -1511,11 +1506,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 FALSE; \
} \
- if (!target || !unit_alive(target->id)) { \
+ if (!target || !unit_alive(target_id)) { \
/* Target unit was destroyed during pre action Lua. */ \
return FALSE; \
}
@@ -1525,7 +1520,7 @@
API_TYPE_ACTION, action_by_number(action), \
API_TYPE_UNIT, actor, \
API_TYPE_TILE, target); \
- if (!actor || !unit_alive(actor->id)) { \
+ if (!actor || !unit_alive(actor_id)) { \
/* Actor unit was destroyed during pre action Lua. */ \
return FALSE; \
}
@@ -1535,7 +1530,7 @@
API_TYPE_ACTION, action_by_number(action), \
API_TYPE_UNIT, actor, \
API_TYPE_TILE, target); \
- if (!actor || !unit_alive(actor->id)) { \
+ if (!actor || !unit_alive(actor_id)) { \
/* Actor unit was destroyed during pre action Lua. */ \
return FALSE; \
}
@@ -2024,7 +2019,7 @@
{
struct action *blocker;
- if (!punit || !unit_alive(punit->id)) {
+ if (!punit) {
/* The actor is dead. */
return FALSE;
}
@@ -2068,11 +2063,11 @@
struct city *pcity)
{
/* Sanity check: The actor is still alive. */
- if (!punit || !unit_alive(punit->id)) {
+ if (!punit) {
return FALSE;
}
- if (!pcity || !city_exist(pcity->id)) {
+ if (!pcity) {
/* City was destroyed during pre action Lua. */
return FALSE;
}
@@ -2176,11 +2171,11 @@
struct city *pcity)
{
/* Sanity check: The actor is still alive. */
- if (!punit || !unit_alive(punit->id)) {
+ if (!punit) {
return FALSE;
}
- if (!pcity || !city_exist(pcity->id)) {
+ if (!pcity) {
/* City was destroyed during pre action Lua. */
return FALSE;
}
@@ -2236,11 +2231,6 @@
int size;
struct player *nationality;
struct player *towner;
-
- /* Sanity check: The actor is still alive. */
- if (!unit_alive(punit->id)) {
- return FALSE;
- }
towner = tile_owner(ptile);
@@ -2465,7 +2455,7 @@
struct player *pplayer = unit_owner(punit);
struct city *pcity = tile_city(ptile);
- if (!punit || !unit_alive(punit->id)) {
+ if (!punit) {
/* Actor unit was destroyed during pre action Lua. */
return FALSE;
}
@@ -2560,7 +2550,7 @@
{
struct city *pcity;
- if (!punit || !unit_alive(punit->id)) {
+ if (!punit) {
/* Actor unit was destroyed during pre action Lua. */
return FALSE;
}
@@ -2631,7 +2621,7 @@
return FALSE;
}
- if (!tgt_city || !city_exist(tgt_city->id)) {
+ if (!tgt_city) {
/* City was destroyed during pre action Lua. */
return FALSE;
}
@@ -2644,7 +2634,7 @@
return FALSE;
}
- if (!act_unit || !unit_alive(act_unit->id)) {
+ if (!act_unit) {
/* Actor unit was destroyed during pre action Lua. */
return FALSE;
}
@@ -3217,11 +3207,6 @@
return FALSE;
}
- /* Sanity check: The actor is still alive. */
- if (!unit_alive(punit->id)) {
- return FALSE;
- }
-
/* Sanity check: The target city still exists. */
if (NULL == pcity_dest) {
return FALSE;
@@ -3319,11 +3304,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
