<URL: http://bugs.freeciv.org/Ticket/Display.html?id=39562 >
On 14/08/2007, Marko Lindqvist <[EMAIL PROTECTED]> wrote:
>
> In current trunk bad choice structures cause illegal memory access
> (old iterators were more robust, but at the same time they hide bugs).
> This patch makes easier to hunt those choice problems down.
This version has even stricter checking. It's not enough that id is
less or equal to maximum Freeciv can handle, but it has to be less or
equal to maximum in currently used ruleset.
This is meant for trunk only. Due to #39563, I assume that trunk
never executes code paths touched by this patch. Successful testing
(new asserts not causing trouble) with current trunk proves nothing.
So I have tested this (with required modifications) mainly in S2_1.
- ML
diff -Nurd -X.diff_ignore freeciv/ai/aicity.c freeciv/ai/aicity.c
--- freeciv/ai/aicity.c 2007-08-13 20:51:05.000000000 +0300
+++ freeciv/ai/aicity.c 2007-08-29 12:37:59.000000000 +0300
@@ -1380,7 +1380,7 @@
}
if (pcity->ai.choice.want != 0) {
- ASSERT_REAL_CHOICE_TYPE(pcity->ai.choice.type);
+ ASSERT_CHOICE(pcity->ai.choice);
CITY_LOG(LOG_DEBUG, pcity, "wants %s with desire %d.",
is_unit_choice_type(pcity->ai.choice.type)
@@ -1532,7 +1532,7 @@
/* Not dealing with this city a second time */
pcity->ai.choice.want = 0;
- ASSERT_REAL_CHOICE_TYPE(bestchoice.type);
+ ASSERT_CHOICE(bestchoice);
/* Try upgrade units at danger location (high want is usually danger) */
if (pcity->ai.urgency > 1) {
@@ -1692,6 +1692,7 @@
game.info.turn + myrand(RECALC_SPEED) + RECALC_SPEED;
}
TIMING_LOG(AIT_CITY_SETTLERS, TIMER_STOP);
+ ASSERT_CHOICE(pcity->ai.choice);
} city_list_iterate_end;
city_list_iterate(pplayer->cities, pcity) {
diff -Nurd -X.diff_ignore freeciv/common/city.h freeciv/common/city.h
--- freeciv/common/city.h 2007-08-13 20:51:03.000000000 +0300
+++ freeciv/common/city.h 2007-08-29 12:40:38.000000000 +0300
@@ -150,25 +150,26 @@
enum choice_type {
CT_NONE = 0,
- CT_BUILDING = 0,
+ CT_BUILDING = 1,
CT_CIVILIAN,
CT_ATTACKER,
CT_DEFENDER,
CT_LAST
};
-/* FIXME:
-
- This should detect also cases where type is just initialized with
- CT_NONE (probably in order to silence compiler warnings), but no real value
- is given. You have to change value of CT_BUILDING into 1 before you
- can add this check. It's left this way for now, is case hardcoded
- value 0 is still used somewhere instead of CT_BUILDING.
-
- -- Caz
-*/
-#define ASSERT_REAL_CHOICE_TYPE(type) \
- assert(type >= 0 && type < CT_LAST /* && type != CT_NONE */ );
+#define ASSERT_CHOICE(c) \
+ do { \
+ if ((c).want > 0) { \
+ assert((c).type >= 0 && (c).type < CT_LAST && (c).type != CT_NONE);\
+ if ((c).type == CT_BUILDING) { \
+ int _iindex = improvement_index((c).value.building); \
+ assert(_iindex >= 0 && _iindex < game.control.num_impr_types); \
+ } else { \
+ int _uindex = utype_index((c).value.utype); \
+ assert(_uindex >= 0 && _uindex < game.control.num_unit_types); \
+ } \
+ } \
+ } while(0);
struct ai_choice {
enum choice_type type;
_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
