http://bugs.freeciv.org/Ticket/Display.html?id=40079 >
While poking at the memory problems, tried moving city ids to avoid the
possible overrun, and was checking how ids were allocated. Discovered that
the running game never marks the used id numbers! And never checks they've
run out of id numbers!
Problem goes back as far as 2.0 (didn't look any farther back).
When savegames are loaded, the id used bits *are* reserved. But the counter
starts again at 100, so reloaded games will not have the same new ids after
that point in the game.
For Leave and Load games, the id counter and id used bits were not reset, so
the new ids start where the previous game finished.
Worse, because two different games could be loaded, the various saved bits
from the previous games are merged, further confounding the issues.
Although this may not be a problem in most games, for large or long games
some units' numbers may be duplicated. And it makes direct comparisons of
saved games absolutely impossible (unless both start at the beginning).
Redid the entire scheme (with symbols)! Set the 0th bit in the used array,
instead of testing against 0 every call. A tiny improvement in efficiency.
I'm now using server_game_init() properly, so the rulesets are no longer
loaded twice for --file games. Another small improvement in efficiency
Left in my debugging swaps of name and/or id for city and unit.
Other minor cleanup.
Index: server/srv_main.c
===
--- server/srv_main.c (revision 14380)
+++ server/srv_main.c (working copy)
@@ -129,10 +129,8 @@
*/
bool force_end_of_sniff;
-/* this counter creates all the id numbers used */
-/* use get_next_id_number() */
-static unsigned short global_id_counter=100;
-static unsigned char used_ids[8192]={0};
+#define IDENTITY_NUMBER_SIZE (1+MAX_UINT16)
+static unsigned char identity_numbers_used[IDENTITY_NUMBER_SIZE/8]={0};
/* server initialized flag */
static bool has_been_srv_init = FALSE;
@@ -196,7 +194,7 @@
init_character_encodings(FC_DEFAULT_DATA_ENCODING, FALSE);
/* Initialize callbacks. */
- game.callbacks.unit_deallocate = dealloc_id;
+ game.callbacks.unit_deallocate = identity_number_release;
/* done */
return;
@@ -1076,37 +1074,43 @@
/**
...
**/
-void dealloc_id(int id)
+void identity_number_release(int id)
{
- used_ids[id/8]&= 0xff ^ (1<<(id%8));
+ identity_numbers_used[id/8] &= 0xff ^ (1<<(id%8));
}
/**
...
**/
-static bool is_id_allocated(int id)
+void identity_number_reserve(int id)
{
- return TEST_BIT(used_ids[id / 8], id % 8);
+ identity_numbers_used[id/8] |= (1<<(id%8));
}
/**
...
**/
-void alloc_id(int id)
+static bool identity_number_is_used(int id)
{
- used_ids[id/8]|= (1<<(id%8));
+ return TEST_BIT(identity_numbers_used[id/8], id%8);
}
/**
-...
+ Truncation of unsigned short wraps at 65K, skipping VISION_SITE_NONE (0)
+ Setup in server_game_init()
**/
+int identity_number(void)
+{
+ int retries = 0;
-int get_next_id_number(void)
-{
- while (is_id_allocated(++global_id_counter) || global_id_counter == 0) {
-/* nothing */
+ while (identity_number_is_used(++server.identity_number)) {
+/* try again */
+if (++retries >= IDENTITY_NUMBER_SIZE) {
+ die("exhausted city and unit numbers!");
+}
}
- return global_id_counter;
+ identity_number_reserve(server.identity_number);
+ return server.identity_number;
}
/**
@@ -1934,9 +1938,12 @@
#endif /* HAVE_AUTH */
/* load a saved game */
- if (srvarg.load_filename[0] != '\0') {
-(void) load_command(NULL, srvarg.load_filename, FALSE);
- }
+ if ('\0' == srvarg.load_filename[0]
+ || !load_command(NULL, srvarg.load_filename, FALSE)) {
+/* Rulesets are loaded on game initialization, but may be changed later
+ * if /load or /rulesetdir is done. */
+load_rulesets();
+ }
if(!(srvarg.metaserver_no_send)) {
freelog(LOG_NORMAL, _("Sending info to metaserver [%s]"),
@@ -2131,13 +2138,14 @@
**/
void server_game_init(void)
{
- game_init();
-
+ /* was redundantly in game_load() */
server.nbarbarians = 0;
+ server.identity_number = IDENTITY_NUMBER_START;
- /* Rulesets are loaded on game initialization, but may be
