Skip to content

Stop using assert() for checking return value from OS functions. #37

@troglobit

Description

@troglobit

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:

  1. On invalid input to a public API the API should return error. E.g, NULL or non-zero with errno=EINVAL
  2. On internal problems, e.g. allocating RAM, the API should return NULL or non-zero with errno set to represent the error condition, e.g. ENOMEM
  3. 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 set NDEBUG. Maybe even use --enable-maintainer-mode, or the reverse of that perhaps: --disable-asserts to the configure script for this.

Comments?

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions