On Wed, Nov 27, 2019 at 10:38:41AM +0100, Richard Levitte wrote: > 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?
My recollection was that we had some reasonable support for this "middle ground" approach. I don't personally have strong enough feelings to advocate a specific position, and will try to adhere to the group consensus going forward. -Ben