Checking new'ed pointers for NULL

[Mateusz Loskot]

Hi,

I'd like to ask about the rationale of some testing pointers for NULL
I've noticed in FDO source code.

There are a few files, especially in fdordbms project
where pointers returned by 'operator new' are tested for NULL.
Here is a sample from FDO sources:

FdoPtr<FdoEnvelopeImpl> envelope = new FdoEnvelopeImpl();
if (envelope == NULL)
   throw FdoException::Create(
            FdoException::NLSGetMessage(FDO_NLSID(FDO_1_BADALLOC)));



My question is what's the reason of these tests?
The only reason I can see is requirement to support old
C++ compilers that does not do automatic nullptr tests
in allocation functions.
(or compilers without exceptions support, e.g. on embedded systems,
but that's not the case here, because FDO uses exceptions extensively)

According to C++ Standard (3.7.3.1 Allocation functions,
3.7.3.1 Allocation functions) 'operator new' is binding to following
requirement:

"Required behavior: Return a non-null pointer to suitably aligned
storage (3.7.3), or else throw a bad_alloc exception. This requirement
is binding on a replacement version of this function."

It means 'operator new' never returns null pointer if fails to allocate
memory (unlike malloc in C), but throw std::bad_alloc exception.

The only version that returns null pointer on allocation failure is
nothrow version of 'operator new', but it should be used explicitly.



In other words, checking new'ed poniters for null instead of catching
std::bad_alloc exception should be considered as a BUG.
The 'if' clause is never executed, because on allocation failure the
std::bad_alloc exception thrown by new will skip the control to matching
catch() clause (if is provided) or will call terminate function and
program will exit with error.

So, all those null pointer tests are not needed but IMHO tedious because
they make code less readable.



The standard compliant and correct verion of snippet presented above is:

try
{
   FdoPtr<FdoEnvelopeImpl> envelope = new FdoEnvelopeImpl;
}
catch (std::bad_alloc &b)
{
   // If FdoException is prefered, re-throw it here
   throw FdoException::Create(
         FdoException::NLSGetMessage(FDO_NLSID(FDO_1_BADALLOC)));
}

Another option, is to use nothrow version of new, keep test for null
and throw FdoException:

FdoPtr<FdoEnvelopeImpl> envelope = new(std:::nothrow) FdoEnvelopeImpl;
if (envelope == NULL)
{
   throw FdoException::Create(
            FdoException::NLSGetMessage(FDO_NLSID(FDO_1_BADALLOC)));
}



Here I grepped out a list of files which include the null pointer test
discussed above:

Fdo/Unmanaged/Src/Geometry/EnvelopeImpl.cpp
Fdo/Unmanaged/Src/Spatial/MathUtility.cpp
Providers/GenericRdbms/Src/Fdo/Filter/FdoRdbmsFilterProcessor.cpp
Providers/GenericRdbms/Src/Fdo/Pvc/FdoRdbmsPvcInsertHandler.cpp
Providers/GenericRdbms/Src/Fdo/Connection/FdoRdbmsPropertyDictionary.cpp
Providers/GenericRdbms/Src/Fdo/Schema/FdoRdbmsSchemaUtil.cpp


p.s. I decided to not to report a defect but push it for
discussion first.

Best regards
-- 
Mateusz Loskot
http://mateusz.loskot.net