Closed patrislav1 closed 6 years ago
Quick thoughts on this:
CMake is supposed to define NDEBUG for Release builds: https://stackoverflow.com/questions/34302265/does-cmake-build-type-release-imply-dndebug I'd be wary of fiddling with NDEBUG directly when there's already a user expectation. Using cmake's internal functionality here also means you don't need to put a guard around the flag addition to protect compilers e.g. MSVC that use a different command line syntax.
CMake here isn't exporting decode_body.h
and vdm.h
as public headers, so they won't e.g. be included in cmake-driven system installs. I'd maybe consider merging into a single aisxx.h
header that carries the user-facing C++ api, and install this? I can look later if seems sensible.
Re asserts, it would still be nice to terminate the message decode if one should trigger but is removed by NDEBUG, to avoid any possible UB. Perhaps it would work to have a new AIS_STATUS code for this, and use the normal error path? Again can look later if seems sensible.
I don't really know or use CMake. Pull requests welcome. I would especially appreciate small ones that focus on an individual point. I'm off battling the large pile of fuzzer generated trouble, but that's in the land a bazel built code.