CFX_Object's current implementation hurts more than helps.

[GoogleCodeExporter]

There's too much code in pdfium that continues in face of OOM via checking for 
NULL returns rather than throwing/terminating.  One of the culprits is 
CFX_Object in fx_memory.h.

As I understand it, the current implementation is:

    void* operator new (size_t size)
    {
        return malloc(size);
    }

and there's a helper macro, typically:

#define FX_NEW new

This code is technically wrong per the spec unless the particular "malloc"
implementation throws rather than returning NULL. Thus, for a throwing malloc, 
all the checks sprinkled throughout the code for NULL following FX_NEW would be 
pointless since we'd never get NULL back.

But it's actually a non-throwing malloc against which this seems to be
built[1]. In that case, calling FX_NEW will segv in the constructor against a 
NULL "this", unless the class initializes no members and contains no virtual 
functions, in which case it may happen to "work".  So checking for NULL returns 
after FX_NEW for most classes is pointless since we'll have already have 
segv'd.  Creative folks may then decide that they'll simply never write 
constructors and instead make explicit Init() methods, and avoid virtual 
functions. But I digress ... 

To get the behaviour the developers intended, you'd need the following forms:

    void* operator new (std::size_t size, const std::nothrow_t& nothrow_value) throw()
    {
        return malloc(size);
    }

#define FX_NEW new (std::nothrow) 

But then you'd have to check every single return, and this feels less safe that 
the current situation.  

CFX_Object be kept around only to facilitate a move to a different allocator 
someday, and probably should be gutted.  At which point we can remove many 
needless checks ...

------
1. Without actually looking to be sure.

Original issue reported on code.google.com by tsepez@chromium.org on 12 Mar 2015 at 7:50