Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Review use of assert() #535

Open
ihilt opened this issue Oct 28, 2024 · 0 comments
Open

Review use of assert() #535

ihilt opened this issue Oct 28, 2024 · 0 comments

Comments

@ihilt
Copy link
Contributor

ihilt commented Oct 28, 2024

There are around 67 uses of the assert() macro 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant