<URL: http://bugs.freeciv.org/Ticket/Display.html?id=40080 >

> [wsimpson - Tue Feb 05 13:29:46 2008]:

> I've found a serious overflow bug already.  The ids are unsigned
short, the
> range is 65536 numbers, but ai/aidata.c allocates all players arrays with:
> 
> /* max size of a short */
> #define MAX_NUM_ID 32767
> 
> BV_DEFINE(bv_id, MAX_NUM_ID);
> 
>          BV_SET(ai->stats.diplomat_reservations, pcity->id);

That's really ludicrous.  First of all, unless IDs are recycled there is
no theoretical limit on the highest ID.  There is a limit on the total
number of cities available at once, that being 1/4 the number of tiles
(or possibly equal to the number of tiles, in some cases).  Having a
massive array to index what is basically a hash value is incredibly
wasteful.  Assuming doing a lookup into pcity->ai->diplomat_reservation
boolean value is too slow (which is I assume why the BV is used), it
would be appropriate to change the BV immediately to do a lookup by tile
index.

As for the general change, Mike has a good point that there could be
hidden risks in making this conversion.  If the conversion IS done,
however, it should be as simple as simply removing the pcity->id value
completely, seeing what breaks, and replacing those breakages with the
use of pcity->tile->index.

-jason


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

Reply via email to