nasa / bplib

Apache License 2.0
30 stars 13 forks source link

Clean up assertions/parameter error checks #72

Open jphickey opened 2 years ago

jphickey commented 2 years ago

In general many organizations/coding standards require certain parameter checks to be performed. While checking the range of numeric inputs is reasonable and a good idea to do, since the valid range is often much smaller than the range of the data type, checking a pointer input is generally much more limited - just NULL or not NULL. Unfortunately, NULL is just one possible bad value (and rather unlikely one, if calling code is actually buggy) out of the billions of possible bad pointer values, so its not really all that useful. What's more, if it returns an error code to the caller (e.g. BP_ERROR) then the program will continue to execute, and lose the state of when the bad call was made, making it difficult to debug. But in all likelihood, the program will crash anyway, just at a later time, when the state/root cause may be obfuscated (that is, one typically gets much better/clearer debug information when an error is caught early, rather than letting that propagate through the system, where secondary/tertiary side effects are mixed in).

So in the end, a check for a "NULL" input pointer neither helps debug programs, nor is it likely to "save" the software from catching an exception and crashing (it just defers it).

One possible solution to balance this out is to put checks for "programmer errors" (such as NULL pointers being passed in) into a locally-defined assert-like macro, that has three possible modes of operation:

During development and debug cycles, programmers/developers would compile in the first mode (abort). Once the software is stable and the lifecycle transitions to acceptance testing, the library would typically be compiled in one of the other two modes. For organizations that have have coding standards that require NULL checks on all pointer inputs, the soft error mode can be used which meets this requirement, or for organizations that do not have such a requirement and want to preserve CPU cycles by not checking for conditions that do not occur (making this qualify as "dead code"), this can be compiled out.

In either case, aside for the "developer mode", the bplib software would be configured in the mode it is intended to by flown in (meeting the "test what you fly, fly what you test" mantra).