Author: sveinung
Date: Sat Oct 24 23:33:15 2015
New Revision: 30201
URL: http://svn.gna.org/viewcvs/freeciv?rev=30201&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:
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=30201&r1=30200&r2=30201&view=diff
==============================================================================
--- trunk/server/diplomats.c (original)
+++ trunk/server/diplomats.c Sat Oct 24 23:33:15 2015
@@ -95,19 +95,13 @@
const char *clink;
/* Fetch target city's player. Sanity checks. */
- if (!pcity) {
- return FALSE;
- }
-
+ fc_assert_ret_val(pcity, FALSE);
cplayer = city_owner(pcity);
- if (!cplayer) {
- return FALSE;
- }
+ fc_assert_ret_val(cplayer, FALSE);
/* Sanity check: The actor still exists. */
- if (!pplayer || !pdiplomat) {
- return FALSE;
- }
+ fc_assert_ret_val(pplayer, FALSE);
+ fc_assert_ret_val(pdiplomat, FALSE);
ctile = city_tile(pcity);
clink = city_link(pcity);
@@ -179,16 +173,17 @@
struct traderoute_packet_list *routes;
/* Fetch target city's player. Sanity checks. */
- if (!pcity) {
- return FALSE;
- }
-
- cplayer = city_owner (pcity);
- if ((cplayer == pplayer) || !cplayer)
- return FALSE;
+ fc_assert_ret_val(pcity, FALSE);
+ cplayer = city_owner(pcity);
+ fc_assert_ret_val(cplayer, FALSE);
/* Sanity check: The actor still exists. */
- if (!pplayer || !pdiplomat) {
+ fc_assert_ret_val(pplayer, FALSE);
+ fc_assert_ret_val(pdiplomat, FALSE);
+
+ /* Sanity check: The target is foreign. */
+ if (cplayer == pplayer) {
+ /* Nothing to do to a domestic target. */
return FALSE;
}
@@ -293,17 +288,17 @@
struct player *cplayer;
/* Fetch target city's player. Sanity checks. */
- if (!pcity) {
- return FALSE;
- }
-
+ fc_assert_ret_val(pcity, FALSE);
cplayer = city_owner(pcity);
- if ((cplayer == pplayer) || !cplayer) {
- return FALSE;
- }
+ fc_assert_ret_val(cplayer, FALSE);
/* Sanity check: The actor still exists. */
- if (!pplayer || !pdiplomat) {
+ fc_assert_ret_val(pplayer, FALSE);
+ fc_assert_ret_val(pdiplomat, FALSE);
+
+ /* Sanity check: The target is foreign. */
+ if (cplayer == pplayer) {
+ /* Nothing to do to a domestic target. */
return FALSE;
}
@@ -364,19 +359,13 @@
struct player *uplayer;
/* Fetch target unit's player. Sanity checks. */
- if (!pvictim) {
- return FALSE;
- }
-
+ fc_assert_ret_val(pvictim, FALSE);
uplayer = unit_owner(pvictim);
- if (!uplayer) {
- return FALSE;
- }
+ fc_assert_ret_val(uplayer, FALSE);
/* Sanity check: The actor still exists. */
- if (!pplayer || !pdiplomat) {
- return FALSE;
- }
+ fc_assert_ret_val(pplayer, FALSE);
+ fc_assert_ret_val(pdiplomat, FALSE);
log_debug("sabotage-unit: unit: %d", pdiplomat->id);
@@ -466,18 +455,13 @@
struct city *pcity;
/* Fetch target unit's player. Sanity checks. */
- if (!pvictim) {
- return FALSE;
- }
+ fc_assert_ret_val(pvictim, FALSE);
uplayer = unit_owner(pvictim);
- if (!uplayer) {
- return FALSE;
- }
+ fc_assert_ret_val(uplayer, FALSE);
/* Sanity check: The actor still exists. */
- if (!pplayer || !pdiplomat) {
- return FALSE;
- }
+ fc_assert_ret_val(pplayer, FALSE);
+ fc_assert_ret_val(pdiplomat, FALSE);
/* Sanity check: The target is foreign. */
if (uplayer == pplayer) {
@@ -619,17 +603,17 @@
/* We have to check arguments. They are sent to us by a client,
so we cannot trust them */
- if (!pcity) {
- return FALSE;
- }
-
- cplayer = city_owner (pcity);
- if ((cplayer == pplayer) || !cplayer) {
- return FALSE;
- }
+ fc_assert_ret_val(pcity, FALSE);
+ cplayer = city_owner(pcity);
+ fc_assert_ret_val(cplayer, FALSE);
/* Sanity check: The actor still exists. */
- if (!pplayer || !pdiplomat) {
+ fc_assert_ret_val(pplayer, FALSE);
+ fc_assert_ret_val(pdiplomat, FALSE);
+
+ /* Sanity check: The target is foreign. */
+ if (cplayer == pplayer) {
+ /* Nothing to do to a domestic target. */
return FALSE;
}
@@ -789,19 +773,13 @@
int revolt_cost;
/* Fetch target civilization's player. Sanity checks. */
- if (!pcity) {
- return FALSE;
- }
-
- cplayer = city_owner (pcity);
- if (!cplayer) {
- return FALSE;
- }
+ fc_assert_ret_val(pcity, FALSE);
+ cplayer = city_owner(pcity);
+ fc_assert_ret_val(cplayer, FALSE);
/* Sanity check: The actor still exists. */
- if (!pplayer || !pdiplomat) {
- return FALSE;
- }
+ fc_assert_ret_val(pplayer, FALSE);
+ fc_assert_ret_val(pdiplomat, FALSE);
/* Sanity check: The target is foreign. */
if (cplayer == pplayer) {
@@ -934,19 +912,13 @@
int count, which;
/* Fetch target city's player. Sanity checks. */
- if (!pcity) {
- return FALSE;
- }
-
- cplayer = city_owner (pcity);
- if (!cplayer) {
- return FALSE;
- }
+ fc_assert_ret_val(pcity, FALSE);
+ cplayer = city_owner(pcity);
+ fc_assert_ret_val(cplayer, FALSE);
/* Sanity check: The actor still exists. */
- if (!pplayer || !pdiplomat) {
- return FALSE;
- }
+ fc_assert_ret_val(pplayer, FALSE);
+ fc_assert_ret_val(pdiplomat, FALSE);
log_debug("sabotage: unit: %d", pdiplomat->id);
@@ -1207,22 +1179,17 @@
int gold_give;
/* Sanity check: The actor still exists. */
- if (!act_player || !act_unit) {
- return FALSE;
- }
+ fc_assert_ret_val(act_player, FALSE);
+ fc_assert_ret_val(act_unit, FALSE);
/* Sanity check: The target city still exists. */
- if (!tgt_city) {
- return FALSE;
- }
+ fc_assert_ret_val(tgt_city, FALSE);
/* Who to steal from. */
tgt_player = city_owner(tgt_city);
/* Sanity check: The target player still exists. */
- if (!tgt_player) {
- return FALSE;
- }
+ fc_assert_ret_val(tgt_player, FALSE);
/* Sanity check: The target is foreign. */
if (tgt_player == act_player) {
@@ -1343,22 +1310,17 @@
const char *tgt_city_link;
/* Sanity check: The actor still exists. */
- if (!act_player || !act_unit) {
- return FALSE;
- }
+ fc_assert_ret_val(act_player, FALSE);
+ fc_assert_ret_val(act_unit, FALSE);
/* Sanity check: The target city still exists. */
- if (!tgt_city) {
- return FALSE;
- }
+ fc_assert_ret_val(tgt_city, FALSE);
/* Who to steal from. */
tgt_player = city_owner(tgt_city);
/* Sanity check: The target player still exists. */
- if (!tgt_player) {
- return FALSE;
- }
+ fc_assert_ret_val(tgt_player, FALSE);
/* Sanity check: The target is foreign. */
if (tgt_player == act_player) {
@@ -1452,22 +1414,17 @@
const char *tgt_city_link;
/* Sanity check: The actor still exists. */
- if (!act_player || !act_unit) {
- return FALSE;
- }
+ fc_assert_ret_val(act_player, FALSE);
+ fc_assert_ret_val(act_unit, FALSE);
/* Sanity check: The target city still exists. */
- if (!tgt_city) {
- return FALSE;
- }
+ fc_assert_ret_val(tgt_city, FALSE);
/* The victim. */
tgt_player = city_owner(tgt_city);
/* Sanity check: The target player still exists. */
- if (!tgt_player) {
- return FALSE;
- }
+ fc_assert_ret_val(tgt_player, FALSE);
tgt_tile = city_tile(tgt_city);
tgt_city_link = city_link(tgt_city);
Modified: trunk/server/unithand.c
URL:
http://svn.gna.org/viewcvs/freeciv/trunk/server/unithand.c?rev=30201&r1=30200&r2=30201&view=diff
==============================================================================
--- trunk/server/unithand.c (original)
+++ trunk/server/unithand.c Sat Oct 24 23:33:15 2015
@@ -286,6 +286,10 @@
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 still exists. */
+ fc_assert_ret_val(pplayer, FALSE);
+ fc_assert_ret_val(punit, 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. */
@@ -2019,10 +2023,9 @@
{
struct action *blocker;
- if (!punit) {
- /* The actor is dead. */
- return FALSE;
- }
+ /* Sanity check: The actor still exists. */
+ fc_assert_ret_val(pplayer, FALSE);
+ fc_assert_ret_val(punit, FALSE);
if (unit_has_type_flag(punit, UTYF_UNDISBANDABLE)) {
/* refuse to kill ourselves */
@@ -2062,15 +2065,12 @@
struct unit *punit,
struct city *pcity)
{
- /* Sanity check: The actor is still alive. */
- if (!punit) {
- return FALSE;
- }
-
- if (!pcity) {
- /* City was destroyed during pre action Lua. */
- 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, FALSE);
/* Add the shields from recycling the unit to the city's current
* production. */
@@ -2171,14 +2171,10 @@
struct city *pcity)
{
/* Sanity check: The actor is still alive. */
- if (!punit) {
- return FALSE;
- }
-
- if (!pcity) {
- /* City was destroyed during pre action Lua. */
- return FALSE;
- }
+ fc_assert_ret_val(punit, FALSE);
+
+ /* Sanity check: The target city still exists. */
+ fc_assert_ret_val(pcity, FALSE);
fc_assert_ret_val(unit_pop_value(punit) > 0, FALSE);
city_size_add(pcity, unit_pop_value(punit));
@@ -2231,6 +2227,10 @@
int size;
struct player *nationality;
struct player *towner;
+
+ /* Sanity check: The actor still exists. */
+ fc_assert_ret_val(pplayer, FALSE);
+ fc_assert_ret_val(punit, FALSE);
towner = tile_owner(ptile);
@@ -2455,10 +2455,9 @@
struct player *pplayer = unit_owner(punit);
struct city *pcity = tile_city(ptile);
- if (!punit) {
- /* Actor unit was destroyed during pre action Lua. */
- return FALSE;
- }
+ /* Sanity check: The actor still exists. */
+ fc_assert_ret_val(pplayer, FALSE);
+ fc_assert_ret_val(punit, FALSE);
log_debug("Start bombard: %s %s to %d, %d.",
nation_rule_name(nation_of_player(pplayer)),
@@ -2550,10 +2549,9 @@
{
struct city *pcity;
- if (!punit) {
- /* Actor unit was destroyed during pre action Lua. */
- return FALSE;
- }
+ /* Sanity check: The actor still exists. */
+ fc_assert_ret_val(pplayer, FALSE);
+ fc_assert_ret_val(punit, FALSE);
log_debug("Start nuclear attack: %s %s against (%d, %d).",
nation_rule_name(nation_of_player(pplayer)),
@@ -2614,30 +2612,17 @@
struct player *tgt_player;
bool try_civil_war = FALSE;
- if (!act_player) {
- /* Someone should be performing the action. */
- fc_assert(act_player);
-
- return FALSE;
- }
-
- if (!tgt_city) {
- /* City was destroyed during pre action Lua. */
- return FALSE;
- }
+ /* Sanity check: The actor still exists. */
+ fc_assert_ret_val(act_player, FALSE);
+ fc_assert_ret_val(act_unit, FALSE);
+
+ /* Sanity check: The target city still exists. */
+ fc_assert_ret_val(tgt_city, FALSE);
tgt_player = city_owner(tgt_city);
- if (!tgt_player) {
- /* How can a city be ownerless? */
- fc_assert(tgt_player);
-
- return FALSE;
- }
-
- if (!act_unit) {
- /* Actor unit was destroyed during pre action Lua. */
- return FALSE;
- }
+
+ /* How can a city be ownerless? */
+ fc_assert_ret_val(tgt_player, FALSE);
/* Save city ID. */
tgt_city_id = tgt_city->id;
@@ -3202,15 +3187,12 @@
{
const char *work;
- if (NULL == punit) {
- /* Probably died or bribed. */
- 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. */
- if (NULL == pcity_dest) {
- return FALSE;
- }
+ fc_assert_ret_val(pcity_dest, FALSE);
pcity_dest->shield_stock += unit_build_shield_cost(punit);
pcity_dest->caravan_shields += unit_build_shield_cost(punit);
@@ -3302,14 +3284,12 @@
struct goods_type *goods;
const char *goods_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
