-
Notifications
You must be signed in to change notification settings - Fork 113
Description
I've noticed that libConfuse relies heavily on assert()
. Personally I think I'd like to see more of a conservative approach to its use. I.e., only use it for internal DBC and not for, e.g. checking return value from malloc()
.
Since libConfuse is a library, I believe we should only assert on things that are clear internal "contract violations", e.g. invalid input to internal functions, or internal fault conditions that simply should not happen. Running out of memory is not one such internal condition in my opinion. Mostly because asserts can be disabled, by defining NDEBUG
. Using assert()
for failed malloc()
is, put simply, laziness ... for a regular desktop system its possibly useful, but for embedded (headless) systems such behavior is rarely desired.
However, I'm a bit hesitant to simply roll out a massive change without posting my proposal here first. So here it is:
- On invalid input to a public API the API should return error. E.g,
NULL
or non-zero witherrno=EINVAL
- On internal problems, e.g. allocating RAM, the API should return
NULL
or non-zero witherrno
set to represent the error condition, e.g.ENOMEM
- Only on internal consistency and sanity checks should the library
assert()
and cause an exit/abort for the calling program -- and this should probably be possible to enable/disable so release binaries setNDEBUG
. Maybe even use--enable-maintainer-mode
, or the reverse of that perhaps:--disable-asserts
to the configure script for this.
Comments?