<URL: http://bugs.freeciv.org/Ticket/Display.html?id=39668 >
Calling script_signal_emit() means that almost anything can happen.
Most definitely cities or units can disappear.
I just glanced where it has been called, and fixed most obvious cases
where some now potentially invalid pointer was used immediately after
script_signal_emit().
I left one case out of this patch. One in treaty handling needs more
inspection.
- ML
diff -Nurd -X.diff_ignore freeciv/common/city.c freeciv/common/city.c
--- freeciv/common/city.c 2007-09-02 07:14:07.000000000 +0300
+++ freeciv/common/city.c 2007-09-04 17:48:27.000000000 +0300
@@ -2504,3 +2504,16 @@
unit_list_free(pcity->units_supported);
free(pcity);
}
+
+/**************************************************************************
+ Check if unit with given id is still alive. Use this before using
+ old unit pointers when unit might have dead.
+**************************************************************************/
+bool city_exist(int id)
+{
+ if (find_city_by_id(id)) {
+ return TRUE;
+ }
+
+ return FALSE;
+}
diff -Nurd -X.diff_ignore freeciv/common/city.h freeciv/common/city.h
--- freeciv/common/city.h 2007-09-02 07:14:07.000000000 +0300
+++ freeciv/common/city.h 2007-09-04 17:48:46.000000000 +0300
@@ -518,6 +518,8 @@
int *pollu_prod, int *pollu_pop, int *pollu_mod);
int city_pollution(const struct city *pcity, int shield_total);
+bool city_exist(int id);
+
/*
* Iterates over all improvements which are built in the given city.
*/
diff -Nurd -X.diff_ignore freeciv/common/unit.c freeciv/common/unit.c
--- freeciv/common/unit.c 2007-08-08 05:52:11.000000000 +0300
+++ freeciv/common/unit.c 2007-09-04 17:37:21.000000000 +0300
@@ -1604,3 +1604,16 @@
return result;
}
+
+/**************************************************************************
+ Check if unit with given id is still alive. Use this before using
+ old unit pointers when unit might have dead.
+**************************************************************************/
+bool unit_alive(int id)
+{
+ if (find_unit_by_id(id)) {
+ return TRUE;
+ }
+
+ return FALSE;
+}
diff -Nurd -X.diff_ignore freeciv/common/unit.h freeciv/common/unit.h
--- freeciv/common/unit.h 2007-08-08 05:52:11.000000000 +0300
+++ freeciv/common/unit.h 2007-09-04 17:35:17.000000000 +0300
@@ -318,4 +318,6 @@
enum unit_upgrade_result get_unit_upgrade_info(char *buf, size_t bufsz,
const struct unit *punit);
+bool unit_alive(int id);
+
#endif /* FC__UNIT_H */
diff -Nurd -X.diff_ignore freeciv/server/cityturn.c freeciv/server/cityturn.c
--- freeciv/server/cityturn.c 2007-08-13 22:23:44.000000000 +0300
+++ freeciv/server/cityturn.c 2007-09-04 18:07:44.000000000 +0300
@@ -465,6 +465,7 @@
bool have_square;
int savings_pct = granary_savings(pcity), new_food;
bool rapture_grow = city_rapture_grow(pcity); /* check before size increase! */
+ int saved_id = pcity->id;
if (!city_can_grow_to(pcity, pcity->size + 1)) { /* need improvement */
if (get_current_construction_bonus(pcity, EFT_SIZE_ADJ, RPT_CERTAIN) > 0
@@ -522,8 +523,10 @@
_("%s grows to size %d."), pcity->name, pcity->size);
script_signal_emit("city_growth", 2,
API_TYPE_CITY, pcity, API_TYPE_INT, pcity->size);
-
- sanity_check_city(pcity);
+ if (city_exist(saved_id)) {
+ /* Script didn't destroy this city */
+ sanity_check_city(pcity);
+ }
sync_cities();
}
@@ -615,6 +618,7 @@
{
bool success = FALSE;
int i;
+ int saved_id = pcity->id;
if (worklist_is_empty(&pcity->worklist))
/* Nothing in the worklist; bail now. */
@@ -624,6 +628,12 @@
while (TRUE) {
struct city_production target;
+ if (!city_exist(saved_id)) {
+ /* Some script has removed useless city that cannot build
+ * what it is told to! */
+ return FALSE;
+ }
+
/* What's the next item in the worklist? */
if (!worklist_peek_ith(&pcity->worklist, &target, i))
/* Nothing more in the worklist. Ah, well. */
@@ -661,15 +671,16 @@
in the worklist, not its obsolete-closure
new_target. */
utype_name_translation(ptarget));
- script_signal_emit("unit_cant_be_built", 3,
- API_TYPE_UNIT_TYPE, ptarget,
- API_TYPE_CITY, pcity,
- API_TYPE_STRING, "never");
+
/* Purge this worklist item. */
worklist_remove(&pcity->worklist, i-1);
/* Reset i to index to the now-next element. */
i--;
-
+
+ script_signal_emit("unit_cant_be_built", 3,
+ API_TYPE_UNIT_TYPE, ptarget,
+ API_TYPE_CITY, pcity,
+ API_TYPE_STRING, "never");
continue;
} else {
/* Yep, we can go after new_target instead. Joy! */
@@ -694,16 +705,16 @@
"Purging..."),
pcity->name,
get_impr_name_ex(pcity, ptarget->index));
- script_signal_emit("building_cant_be_built", 3,
- API_TYPE_BUILDING_TYPE, ptarget,
- API_TYPE_CITY, pcity,
- API_TYPE_STRING, "never");
/* Purge this worklist item. */
worklist_remove(&pcity->worklist, i-1);
/* Reset i to index to the now-next element. */
i--;
-
+
+ script_signal_emit("building_cant_be_built", 3,
+ API_TYPE_BUILDING_TYPE, ptarget,
+ API_TYPE_CITY, pcity,
+ API_TYPE_STRING, "never");
continue;
}
@@ -822,7 +833,14 @@
}
break;
}
+
+ if (!city_exist(saved_id)) {
+ /* Some script has removed city */
+ return FALSE;
+ }
+
} requirement_vector_iterate_end;
+
if (!known) {
/* This shouldn't happen...
FIXME: make can_build_improvement() return a reason enum. */
@@ -1056,6 +1074,7 @@
int mod;
Impr_type_id id = pcity->production.value;
struct impr_type *building = improvement_by_number(id);
+ int saved_id = pcity->id;
if (!pcity->production.is_unit
&& improvement_has_flag(pcity->production.value, IF_GOLD)) {
@@ -1128,6 +1147,11 @@
API_TYPE_BUILDING_TYPE, improvement_by_number(id),
API_TYPE_CITY, pcity);
+ if (!city_exist(saved_id)) {
+ /* Script removed city */
+ return FALSE;
+ }
+
/* Call this function since some buildings may change the
* the vision range of a city */
city_refresh_vision(pcity);
@@ -1192,19 +1216,24 @@
_("%s is building %s, which is no longer available."),
pcity->name,
utype_name_translation(utype));
- script_signal_emit("unit_cant_be_built", 3,
- API_TYPE_UNIT_TYPE, utype,
- API_TYPE_CITY, pcity,
- API_TYPE_STRING, "unavailable");
+
+ /* Log before signal emitting, so pointers are certainly valid */
freelog(LOG_VERBOSE, "%s's %s tried to build %s, which is not available.",
pplayer->name,
pcity->name,
utype_rule_name(utype));
+
+ script_signal_emit("unit_cant_be_built", 3,
+ API_TYPE_UNIT_TYPE, utype,
+ API_TYPE_CITY, pcity,
+ API_TYPE_STRING, "unavailable");
+
return TRUE;
}
if (pcity->shield_stock >= unit_build_shield_cost(utype)) {
int pop_cost = unit_pop_value(utype);
struct unit *punit;
+ int saved_city_id = pcity->id;
/* Should we disband the city? -- Massimo */
if (pcity->size == pop_cost
@@ -1254,15 +1283,17 @@
script_signal_emit("unit_built",
2, API_TYPE_UNIT, punit, API_TYPE_CITY, pcity);
- /* Done building this unit; time to move on to the next. */
- choose_build_target(pplayer, pcity);
+ if (city_exist(saved_city_id)) {
+ /* Done building this unit; time to move on to the next. */
+ choose_build_target(pplayer, pcity);
+ }
}
return TRUE;
}
/**************************************************************************
-return 0 if the city is removed
-return 1 otherwise
+return FALSE if the city is removed
+return TRUE otherwise
**************************************************************************/
static bool city_build_stuff(struct player *pplayer, struct city *pcity)
{
@@ -1545,6 +1576,7 @@
struct tile *ptile = pcity->tile;
struct city *rcity=NULL;
struct unit_type *utype = utype_by_number(pcity->production.value);
+ int saved_id = pcity->id;
/* find closest city other than pcity */
rcity = find_closest_owned_city(pplayer, ptile, FALSE, pcity);
@@ -1560,7 +1592,12 @@
API_TYPE_UNIT_TYPE, utype,
API_TYPE_CITY, pcity,
API_TYPE_STRING, "pop_cost");
- return FALSE;
+ if (!city_exist(saved_id)) {
+ /* Script decided to remove even the last city */
+ return TRUE;
+ } else {
+ return FALSE;
+ }
}
(void) create_unit(pplayer, ptile, utype,
diff -Nurd -X.diff_ignore freeciv/server/unittools.c freeciv/server/unittools.c
--- freeciv/server/unittools.c 2007-09-02 15:37:58.000000000 +0300
+++ freeciv/server/unittools.c 2007-09-04 17:41:42.000000000 +0300
@@ -2317,6 +2317,7 @@
struct player *pplayer = unit_owner(punit);
bool ok = TRUE;
int hut_chance = myrand(12);
+ int saved_id = punit->id;
if (game.info.hut_overflight == OVERFLIGHT_NOTHING && is_air_unit(punit)) {
return ok;
@@ -2340,32 +2341,36 @@
script_signal_emit("hut_enter", 1, API_TYPE_UNIT, punit);
- switch (hut_chance) {
- case 0:
- hut_get_gold(punit, 25);
- break;
- case 1: case 2: case 3:
- hut_get_gold(punit, 50);
- break;
- case 4:
- hut_get_gold(punit, 100);
- break;
- case 5: case 6: case 7:
- hut_get_tech(punit);
- break;
- case 8: case 9:
- if (num_role_units(L_HUT) != 0) {
- hut_get_mercenaries(punit);
- } else {
- hut_get_gold(punit, 25);
+ if (unit_alive(saved_id)) {
+ /* Script didn't cause unit death */
+
+ switch (hut_chance) {
+ case 0:
+ hut_get_gold(punit, 25);
+ break;
+ case 1: case 2: case 3:
+ hut_get_gold(punit, 50);
+ break;
+ case 4:
+ hut_get_gold(punit, 100);
+ break;
+ case 5: case 6: case 7:
+ hut_get_tech(punit);
+ break;
+ case 8: case 9:
+ if (num_role_units(L_HUT) != 0) {
+ hut_get_mercenaries(punit);
+ } else {
+ hut_get_gold(punit, 25);
+ }
+ break;
+ case 10:
+ ok = hut_get_barbarians(punit);
+ break;
+ case 11:
+ hut_get_city(punit);
+ break;
}
- break;
- case 10:
- ok = hut_get_barbarians(punit);
- break;
- case 11:
- hut_get_city(punit);
- break;
}
send_player_info(pplayer, pplayer); /* eg, gold */
@@ -2724,6 +2729,7 @@
struct unit *ptransporter = NULL;
struct vision *old_vision = punit->server.vision;
struct vision *new_vision;
+ int saved_id = punit->id;
conn_list_do_buffer(pplayer->connections);
@@ -2890,10 +2896,19 @@
} square_iterate_end;
handle_unit_move_consequences(punit, psrctile, pdesttile);
+
+ /* FIXME: Should signal emit be after sentried units have been
+ * waken up in case script causes unit death. */
script_signal_emit("unit_moved", 3,
API_TYPE_UNIT, punit,
API_TYPE_TILE, psrctile,
API_TYPE_TILE, pdesttile);
+
+ if (!unit_alive(saved_id)) {
+ /* Script caused unit to die */
+ return FALSE;
+ }
+
wakeup_neighbor_sentries(punit);
if (!unit_survive_autoattack(punit)) {
conn_list_do_unbuffer(pplayer->connections);
_______________________________________________
Freeciv-dev mailing list
[email protected]
https://mail.gna.org/listinfo/freeciv-dev
