Follow-up Comment #6, bug #15305 (project freeciv):
Here is my second attempt at a patch. This time around, the idea is to make
memory management for city.c : city_map_index and map.c :
map.iterate_outwards_indices work exactly the same way as it does for
map.tiles.
This time calls to realloc are replaced with calls to malloc, and some
asserts are added. The asserts are simply to clarify the intent of the code,
and I have a strong argument as to why the assertions always hold. Here
goes.
The memory for map.tiles, city_map_index, and map.iterate_outwards_indices is
allocated in map_allocate(), and with the patch it is released in map_free().
There is no scenario in which one of these should be allocated and any of the
others should not. Hence the patch is correct assuming the calls to
map_allocate() and map_free() are themselves correct. Are they?
Well, there are 3 scenarios in which map_allocate is called. I have detailed
notes about this, but to make a long story short:
* The server calls map_allocate when a saved game is loaded
* The server calls map_allocate when a new map is generated
* The client calls map_allocate when it receives a PACKET_MAP_INFO
The map_free is function called in game_free, which is called in
server_game_free, and I don't think there is any indication right now there
is any problem with the server properly calling server_game_free after a game
is played. Hence the server is OK.
The client is interesting because map_free is sometimes called before
map_allocate in the packet handler. These calls are OK because map_free does
nothing if map_allocate hasn't been called yet. The packet handling code also
performs a redundant check of the same condition in this case using
map_is_empty.
Anyway, the calls to map_free guarantee map_allocate won't leak if the server
sends multiple MAP_INFO packets during a game for some reason. Additionally,
the client calls game_reset or game_free (which call map_free) when starting
new games or quitting, hence those cases are handled properly as well.
To sum up, map_allocate and map_free are called at appropriate times, so it
makes sense to do all the memory management for the map during those calls.
That's what this patch does.
JKL
(file #8007)
_______________________________________________________
Additional Item Attachment:
File name: map_allocate_memory_leak2.patch Size:4 KB
_______________________________________________________
Reply to this item at:
<http://gna.org/bugs/?15305>
_______________________________________________
Message sent via/by Gna!
http://gna.org/
_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
