Conversation
fester
left a comment
fester
left a comment
There was a problem hiding this comment.
Could you please elaborate why did you added the two main_loop_* functions? Wasn't it enough just to catch a bad_alloc somewhere in the main()?
| @@ -150,30 +201,6 @@ static int subcommand_dem2tintiles(bool need_help, | |||
| if("zemlya" == meshing_method || "terra" == meshing_method) | |||
| { | |||
| max_error = input_raster->get_cell_size(); | |||
There was a problem hiding this comment.
It seems that this line will always erase a max_error value passed throught CLI
There was a problem hiding this comment.
yes, that was the case in the original code. So I guess that was on purpose? I cannot tell if that was intentional or not.
There was a problem hiding this comment.
ok, completely messed this up. fixed.
src/cmd.cpp
Outdated
| throw po::error(std::string("unknown method ") + meshing_method); | ||
| } | ||
|
|
||
| char* emergency_memory = new char[16384]; |
There was a problem hiding this comment.
Could you please clarify this buffer allocation? Also you don't free it when no bad_alloc happens. In order to be consistent with the rest of codebase, please use unique_ptr instead of a raw pointer.
There was a problem hiding this comment.
The reason for this is, that if an OOM happens, the system might not have enough memory to actually handle and print the error message. Therefore 16k are allocated here, to be freed later (will add the free at the end of the code) to have enough memory to print the error.
There was a problem hiding this comment.
moved this to unique_ptr and added the same logic further below where I forgot to add it
|
I added the two main loop functions to reduce the width of how open the try catch will be. And to separate all the stuff needed to actually run the code from input argument parsing and validation (which could be even more separated IMO). Will address the comments tomorrow. |
adrpar
force-pushed
the
nice_error_on_oom
branch
2 times, most recently
from
c2d3e8d to
e46dc57
Compare
…steps in before that... Signed-off-by: Adrian Partl <adrian.partl@here.com>
adrpar
force-pushed
the
nice_error_on_oom
branch
from
e46dc57 to
d567a88
Compare
| if("zemlya" == meshing_method || "terra" == meshing_method) | ||
| std::unique_ptr<char[]> emergency_memory(new char[16384]); | ||
|
|
||
| try |
There was a problem hiding this comment.
wonder if this has performance cost in our case https://stackoverflow.com/questions/1897940/in-what-ways-do-c-exceptions-slow-down-code-when-there-are-no-exceptions-thown
3 participants
however usually the OOM killer steps in before that...