Check NULL pointers or not...

[Richard Levitte]

Depending on who you're asking, you get different responses.

The topic is: should we check pointers coming from applications before
using them or not?

There are two positions on this:

1. Yes we should so we don't crash processes unnecessarily, and we
   should return an error if pointers are NULL (unless that's valid
   input, such as for the freeing functions).  There is a generic
   error reason code for it, ERR_R_PASSED_NULL_PARAMETER.

2. No we should not, as we don't cater for user programming errors.
   Also, it allows us to catch our own errors early.

For a while, our pendulum was going with option 2, but lately, we seem
to go back to option 1.

Both options have their pros and cons:

1. cons: there's the very real possibility that users don't check for
         errors as they should, and things go wrong further on,
         sometimes in ways where the actual cause is very hard to
         figure out.
   pros: programs will not spuriously crash because we didn't catch an
         internal corner case in our tests.

2. cons: programs may crash, sometimes due to our own mistakes,
         sometimes due to user errors.
   pros: some very subtle bugs are found, either quicker or at all.
         An actual case is PR 7630 [1], which was uncovered because
         there was a crash due to a NULL being used (this was
         ultimately due to users not checking errors).

As a middle ground, and perhaps a way to satify both camps, we could
use ossl_assert() in our checks for input NULL, as follows:

    if (!ossl_assert(ptr != NULL)) {
        ERR_raise(ERR_LIB_WHATEVER, ERR_R_PASSED_NULL_PARAMETER);
        return 0;
    }

This means that in a debug build, there may be crashes due to pointers
being NULL, while in a release build, the caller gets an error.

Thoughts?

Cheers,
Richard

[1] https://github.com/openssl/openssl/pull/7630

-- 
Richard Levitte         levitte at openssl.org
OpenSSL Project         http://www.openssl.org/~levitte/