Remove/cleanup usage of LAL_NDEBUG/NDEBUG in LAL
Description
This MR addresses the use of LAL_NDEBUG
/NDEBUG
in LALSuite code. In most cases, use of these symbols to disable error messages/checking is removed; the performance cost of these checks is likely minimal, compared to the human cost of debugging obscure errors/segfaults (cf. #573 (closed)). Where legitimate use of such symbols remains, NDEBUG
is used. (LAL_NDEBUG
used to be controlled by the ./configure
option --enable/--disable-debug
, but since that option was deprecated in 2016 it has been permanently #undef
ined in LALConfig.h
. It has now been removed entirely.)
LALMalloc.h
The most important use of LAL_NDEBUG
/NDEBUG
was in LALMalloc.h
. This header defined two sets of standard memory allocation functions, LALMalloc()
etc., and XLALMalloc()
etc. These functions are used equivalently in many places in LALSuite, e.g. memory allocated by LALCalloc()
may be freed by XLALFree()
. An important difference is that the LALMalloc()
functions are replaced with malloc()
et al. when NDEBUG
is defined, but the XLALMalloc()
functions are not. This means that NDEBUG
has to be defined consistently, not only when building LAL, but which building any code that links against LAL. The failure for this to happen in the Conda packages (!1929 (merged)) coupled with the mixing of LALMalloc()
and XLALMalloc()
functions in some codes meant that, in one LALPulsar executable, code allocated by LALCalloc()
(which was pointing to calloc()
) was passed to XLALFree()
, generating an abort from LALCheckMemoryLeaks()
.
There is no reason to continue to maintain separate LAL and XLAL versions of the standard memory functions, particularly given these functions are widely assumed to be equivalent but may not be (e.g. when NDEBUG
is defined). And allowing whether LALMalloc()
or malloc()
are used based on a macro in a header (e.g. NDEBUG
) is dangerous, as one cannot enforce that choice consistently in all code that links against LAL.
This MR addresses these 2 points:
LALMalloc.h
now just definesXLALMalloc()
etc. as functions;LALMalloc()
exist only as compatibility macros. (WithinLALMalloc.c
, the implementations ofXLALMalloc()
are those of the formerLALMalloc()
functions.) For each memory function there is a e.g.XLALMallocLong()
version withfile
andline
arguments, anXLALMallocShort()
version without, and aXLALMalloc()
macro which points toXLALMallocLong(..., __FILE__, __LINE__)
.NDEBUG
is no longer used inLALMalloc.h
. Instead, if one really wants to disable the LAL memory functions completely (beyond not setting the memory-tracking bits inLAL_DEBUG_LEVEL
) one can./configure
with--disable-memory-functions
; this compiles LAL with-DLAL_MEMORY_FUNCTIONS_DISABLED
which pointsXLALMalloc()
etc. tomalloc()
etc. insideLALMalloc.c
. Calls tomalloc()
throughXLALMalloc()
will incur an extra function call (sinceXLALMalloc()
is no longer defined away) but this performance penalty is likely minimal. On the other hard, the use of LAL memory functions throughLALMalloc.h
is now guaranteed to be consistent across all code which links against a particularliblal.so
, preventing the kinds of bugs found by #573 (closed).
Aside from the interface changes, the actual operation of the LAL memory functions (e.g. the memory tracking and overflow checking features) are unchanged.
Other uses of LAL_NDEBUG/NDEBUG
These changes have been split into a separate MR; see !1938 (merged)
Remaining uses of NDEBUG
The only remaining use of NDEBUG
after this MR is to enable/disable the LALStatus
macro ASSERT()
, and code associated with using ASSERT()
. Given that ASSERT()
is meant to be the LALStatus
equivalent of the standard C assert()
it's reasonable to use the same macro. (As stated above, LAL_NDEBUG
has been removed completely.)
API Changes and Justification
Backwards Compatible Changes
-
This change does not modify any class/function/struct/type definitions in a public C header file or any Python class/function definitions -
This change adds new classes/functions/structs/types to a public C header file or Python module
Backwards Incompatible Changes
-
This change modifies an existing class/function/struct/type definition in a public C header file or Python module -
This change removes an existing class/function/struct/type from a public C header file or Python module
This breaks the LALMalloc.h
ABI due to the removal of the LALMalloc()
et al. functions, which are now just macros.
In particular, XLALFree()
is no longer a function, so code which needs a pointer to XLALFree()
(e.g. as a destructor function) should instead use XLALFreeShort()
.
The advantage of preventing obscure bugs like #573 (closed) from arising is IMHO worth the cost of the ABI break.