lcm-proj/lcm

Review use of `assert()`

Opened this issue · 0 comments

ihilt commented

There are around 67 uses of the assert() function in LCM non-test code. Given what the man page says about this function,

If the macro NDEBUG is defined at the moment <assert.h> was last
included, the macro assert() generates no code, and hence does
nothing at all. It is not recommended to define NDEBUG if using
assert() to detect error conditions since the software may behave
non-deterministically.

and,

assert() is implemented as a macro; if the expression tested has
side-effects, program behavior will be different depending on
whether NDEBUG is defined. This may create Heisenbugs which go
away when debugging is turned on.

I think it would be a good idea to review them and make sure there are no side-effects introduced, meaning, the code in the call to assert() does nothing other than a comparison. An example of what I'm talking about,

        assert(lcm_ringbuf_used(*ringbuf) > 0);

https://github.com/lcm-proj/lcm/blob/master/lcm/udpm_util.c#L191

If NDEBUG is defined before assert() is called, then this produces no code and lcm_rinbuf_used() is not called. This could produce different behavior between Release and Debug builds.

Pointer dereference seems to be another category of code passed to assert(). This is less of a concern than the first category. But, it would be good to review those uses as well to determine whether pointer deref could produce different results between build types.