Mihail Naydenov - 2016-04-12

I was wondering why does the metadata member was created as fih->metadata = new METADATAMAP;

Seemed wired to allocate standard library containers on the heap.

But there is reason for it, if we store metadata not as pointer, but as by-value member will not be able to use it because it is never constructed, it is never constructed because it's containing struct (FREEIMAGEHEADER) is never constructed - it is malloc-ed and then its fields are assigned.

There is no reason, this has to be the case.

C++ has tools to construct objects into a pre-allocated memory - placement-new.

For instance, if we have metadata member variable by value we can new (&fih->metadata) METADATAMAP; and metadata will happily be constructed and ready for use.
This does not allocate memory and does not trow exceptions!

But why solve this localy, we really should construct FREEIMAGEHEADER instead! Ironically this is more intuitive then the current code:

Before:

FREEIMAGEHEADER *fih = (FREEIMAGEHEADER *)bitmap->data;

After

FREEIMAGEHEADER *fih = new (bitmap->data) FREEIMAGEHEADER; //< no allocations or exceptions here

Done, metadata member variable is ready to use!
Not just that - all future structs and classes we'll add to the header will also be constructed for us.

Now we not only eliminated one 100% useless allocation, as well as needles pointer indirection (which makes the code slower and less intuitive to use), but we also open the door to add constructor, copy constructor and destructor to FREEIMAGEHEADER (making it self-contained), we also added the possibility, as said, to insert other structs and classes to the header or convert existing ones to ones that do their own construction and destruction and alleviated the need to manage them manually.

The two caveats when using placement-new:

1) Memory pointer provided must be with the correct alignment for the object we construct. We have nothing to wary, considering FREEIMAGEHEADER is already 16 byte aligned!

2) We MUST also call the destructor of FREEIMAGEHEADER.

((FREEIMAGEHEADER *)dib->data)->~FREEIMAGEHEADER(); //< we need this now
// delete bitmap ... 
FreeImage_Aligned_Free(dib->data);

However metadata no longer needs to be delete-ted - the destructor takes care of that!

Of course FreeImage_Clone must be changed as well, preferably making use of a copy constructor so we'll be able to just

new (new_dib->data) FREEIMAGEHEADER(*((FREEIMAGEHEADER *)dib->data));

and be done with FREEIMAGEHEADER construction in a single call!

 

Last edit: Mihail Naydenov 2016-04-12