Follow-up Comment #15, bug #18087 (project freeciv):
Attached my version of file #12960, split into three separate patches and
tweaked a little.
The three patches:
* *trunk-S2_3-hugemap-stack-overflow.diff*: The stack overflow fixes. This is
what I propose we apply to S2_3 pre RC1, to deal with bug #17962. _I will do
so if there are no objections in 36h._
** I've reviewed this to check that no memory leaks came in with the
introduction of heap allocation due to returning without freeing. I only
noticed one: in sg_load_map_known()), SAVE_MAP_CHAR() -> sg_failure_ret() ->
sg_check_ret() can return prematurely. This is a leaky pattern and should be
dealt with (raise a ticket), but it's an error condition so I think we can
live without a fix for RC1.
** I've only reviewed the existing changes for correctness; I haven't looked
to see if there are any map-sized stack allocations that have been missed.
** I haven't reviewed the new flooding algorithm in detail. It looks vaguely
plausible.
** As a sanity check, I've played a single-player game with this patch (S2_3)
for a few tens of turns without the world ending. No autogame testing yet.
* *trunk-S2_3-hugemap-colatitude.diff*: This messes with colatitude stuff I
don't fully understand, so it seems riskier to me. Unless someone thinks
otherwise, I suggest we leave it out for 2.3.0-RC1, since it's not necessary
to fix the stack issues. Could add it back to a later 2.3.x.
** This bit of the patch is Matthias; it came in between file #12942 (comment
#4) and file #12947 (comment #5).
** Matthias: do you think this patch is useful/necessary for the map size
limits we have on S2_3?
* *trunk-S2_3-hugemap-biggermaps.diff*: This actually increases the maximum
map sizes. I suggest we leave this for trunk only.
** Aside: I think the correct way to express the relationship between
MAP_MAX_SIZE and MAX_DBV_LENGTH is to publish the latter in bitvector.h and
have a static (compile-time) assert in the map code. But we don't seem to have
a static assert mechanism yet. I may look into it.
My tweaks were:
* Remove memset()s that were redundant due to use of fc_calloc()
* Change free() to FC_FREE() throughout, for form's sake
* Add 'const' to constant arrays in smooth_int_map()
* Rationalise away 'ret' in rand_map_pos_filtered()
(file #13529, file #13530, file #13531)
_______________________________________________________
Additional Item Attachment:
File name: trunk-S2_3-hugemap-stack-overflow.diff Size:16 KB
File name: trunk-S2_3-hugemap-colatitude.diff Size:1 KB
File name: trunk-S2_3-hugemap-biggermaps.diff Size:2 KB
_______________________________________________________
Reply to this item at:
<http://gna.org/bugs/?18087>
_______________________________________________
Message sent via/by Gna!
http://gna.org/
_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
