It seems only bench is compiled with -DNDEBUG defined. The object used in the .a and .so file should be also be built with -DNDEBUG.
Ping @theuni?
The secp256k1.o object should be built with NDEBUG #38
issue sipa opened this issue on June 23, 2014-
sipa commented at 12:55 AM on June 23, 2014: contributor
-
theuni commented at 4:29 PM on June 23, 2014: contributor
This pretty much comes down to maintainer preference. Most packages don't enable NDEBUG, rather they let the user handle it via ./configure CPPFLAGS=-DNDEBUG. In reality, it's usually the distros that handle the above.
In this case, bench hard-coding NDEBUG makes sense, since it's not user-facing. One could possibly argue the same for the tests.
Another option is something like --enable-debug, on by default.
Preference?
-
sipa commented at 12:53 AM on June 25, 2014: contributor
I'd argue that a library whose primary purpose is efficiency can reasonably be expected to not build debug code by default, but maybe that's uncommon practice.
Of course, we could equally just replace assert() by a macro that only has an effect when VERIFY is defined? :)
-
gmaxwell commented at 12:57 AM on June 25, 2014: contributor
We should probably not use assert.h. We should use local debugging macros instead. It would resolve that issue nicely...
-
theuni commented at 4:06 AM on June 25, 2014: contributor
@sipa: I would agree with that. Agree with @gmaxwell as well.
Since assert has performance implications, and is more complicated than simply "die here when debugging, and I guarantee we'll never get here in production", hard-coded -DNDEBUG and a custom secp_assert() with those simple semantics makes sense in this case.
-
- sipa closed this on Aug 28, 2014
