CleverRaven/Cataclysm-DDA — pull request #5984 by BevapDin (2014-02-06 01:51:47+00:00)
Title: fix submap saving, erase memory leaks, ignore incomplete submaps

fixes #5980, at least for that specific save.

The savefile from this report http://smf.cataclysmdda.com/index.php?topic=5251.msg85763#msg85763 is technically invalid, as it contains 59930 submaps. This is against the assumption that mapgen always creates 4 submaps at once (59930==14982*4+2).
Specifically there is a submap (-1,68,0) and (-1,69,0), but no submap (-2,68,0) nor (-2,69,0)

mabuffer.cpp line 226: the position of submap is written, even if the submap does not exists (lookup_submap on the next line returns NULL).

mapbuffer::save_quad will write the submaps (-2,68,0), (-2,69,0), (-1,68,0), (-1,69,0) in that order into the file. Calling lookup_submap for (-2,68,0), the file only contains 3 values, that have just been written, so lookup_submap fails.

Than it looks for (-2,69,0) and now the file contains 6 values (coords of (-2,68,0) and (-2,69,0): 2 lines with 3 numbers each) and the unserialize_submaps thinks that it contains a whole submap so it reads it. But that file starts with the coordinates of (-2,68,0), so the loaded submap is stored as that one and lookup_submap returns NULL ((-2,69,0) still does not exist).

Now the submap (-2,68,0) suddenly exists. The other two submaps are written normally.

Back into `mapbuffer::save`, the loop head (very clever actually) assumes that `mapbuffer::save_quad` deletes all 4 submaps mentioned above, as they have just been saved. The submap (-2,68,0) has not been deleted, it exists now. So it starts the loop again with submap (-2,68,0), which is now the first in the map.

But saved_submaps already contains the position of that submap, so it's ignored and the loop start again. This goes on as the lines 176-189 do not change the state of the loop condition and therefor lead to an infinite loop.

Similar happens again at (94,-430,0), similar reason (not investigated that one).

Solution: don't call lookup_submap when saving submaps, as lookup_submap will load the submap if it's missing (or trying to do so, because all it needs is 5 values, that are loaded into locx, locy, locz, turn, temperature. Then eof is checked, than the rest of the submap is loaded without checking if there is any rest at all - there should be SEEX*SEEY terrain entries, but this is not checked).