Segmentation fault on raise land over WORLD_Z_SIZE

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Go to landscaping tool
2. Keep scrolling up until you reach WORLD_Z_SIZE. (For easier testing, I set 
it to 10)
3. Crash

What is the expected output? What do you see instead?
Expected: Stop raising land.
Got: Segmentation fault

What version of the product are you using? On what operating system?
r599 on Gentoo linux 3.2.12

Please provide any additional information below.
I believe the problem to be in the function Voxel *VoxelStack::GetCreate(int16 
z, bool create),
on the first line (map.cpp:148) because if z >= WORLD_Z_SIZE, it returns NULL 
pointer which then TerrainChanges::ModifyWorld tries to dereference that NULL 
in the following lines (terraform.cpp:507-513).

Original issue reported on code.google.com by mkr...@gmail.com on 11 Dec 2012 at 9:06

[GoogleCodeExporter]

Hmm, yeah, I don't remember implementing code to stop people from doing these 
kinds of things, so a crash seems likely to happen when you push to the 
boundaries.

I expect you can cause chaos with paths and shops in much the same way.

The fix for these problems would be to check before-hand whether an operation 
would cause trouble, and issue an error to the user. First problem is then that 
there is no window to report it to the user :)

Original comment by Alberth2...@gmail.com on 12 Dec 2012 at 8:03

• Changed state: Accepted

[GoogleCodeExporter]

I have created a patch that fixes this. Since I haven't yet had time to study 
the whole game mechanics so I cannot guarantee it won't break something else, 
but to me it looks promising.
Also, there is no problem with the path building. It simply silently refuses to 
go up.

Original comment by mkr...@gmail.com on 12 Dec 2012 at 9:38

Attachments:

• fix_scrolling_up_r599.patch

[GoogleCodeExporter]

Solved the issue one line earlier (by checking the height), but the idea is the 
same.

On the longer time scale, this is the wrong solution though.
- Just refusing to work leaves the user wondering why it does not work. In 
other words, instead of "false" it should return "this corner is too high, this 
shop or path is in the way", and whatever other reason may exist (probably with 
some visual clues about the problem).
- To make the move to network games simpler (assuming that will happen at some 
point in the future), feedback to the user should be kept outside the operation 
itself (and the operation should be split in "test" and "execute" I think, but 
I have not thought about it much).

Somewhat unrelated, currently no money is charged for this operation.

But for now, it fixes the crash.
Thanks for finding the issue and making the patch.

Original comment by Alberth2...@gmail.com on 14 Dec 2012 at 9:49

• Changed state: Fixed

[GoogleCodeExporter]

By the way, the fix is in r603.

Original comment by Alberth2...@gmail.com on 14 Dec 2012 at 9:57