Pass pointer data across multiple functions

I would like review on the following dynamic memory allocation process and suggestions on whether there are any memory leaks. Following code is not real code in use, but I'm trying to understand concepts of memory allocations and de-allocation in different ways.

class ObjMapData
{
private:
//Header Info fields to describe each object actual data type, total elements of actual data type, and lot more describing the object common fields
int itsDataTypeID; // 1 to 20+, each represent one type of data being used
int itsDataType; // 1 to 8, which could be UINT8, INT8, UINT16, INT16, UINT32, INT32
int itsDataSize; // 1 or 2 or 4 bytes
int itsTotalElements; // original data type elements, but not the total bytes of the binary file
//...... other common header information
// body data, (actual data could be UINT8, INT8, UINT16, INT16, UINT32, INT32 type)
char* itsMapData; // size = itsTotalElements*itsDataSize; type cast to various actual data types later to access/populate the data
........
public:
ObjMapData();
ObjMapData& operator = (const ObjMapData& objMyMap);
ObjMapData(const ObjMapData& objMyMap);
~ObjMapData(){if(itsMapData!= NULL) delete[] itsMapData;}
//Following two functions are used to allocate/free memory within the scope of the object so that it can be re-used again for a different data type object
ClearMemory() {if(itsMapData!= NULL) {delete[] itsMapData; itsMapData= NULL}}
void AllocateMemory( unsigned long  nSizeInBytes = 0)
{
if(itsMapData!= NULL)   {delete[] itsMapData; itsMapData= NULL;}
itsMapData= new (std::nothrow) INT8[nSizeInBytes];
}
.......
void SetMapData(char* ptrData) { itsMapData = ptrData;} // or should I use char*&ptrData??
char* GetMapData() const { return itsMapData;}
}

Now can I do the following without any memory leaks?

```
//There are 30+ functions like Function1() to populate the map data of different type of maps using different sources of input data or algorithms
bool Function1(ObjMapData& objMyMap)
{
//populate the ptrDa

Don't do your own memory management. Use the tools provided for you unless you have good reason to do otherwise.

This:

objMyMap.SetMapData((char*)ptrData);

causes undefined behavior. This is because when objMyMap is deleted you delete the pointer as a char pointer (it was created as an int pointer). Thus the runtime memory management will probably get the size of the block wrong.

If you ever use a C-Cast then you are probably doing something wrong. Always use C++ style casts. Then it is easy to spot when you are doing things wrong.

const_cast<>()          // Bad if you casting away constness.
reinterpret_cast<>()    // Bad unless used to interact with C libraries.
static_cast<>()         // Bad if used to cast pointers around
dynamic_cast<>()        // Safe but slow as it is a runtime cast.
                        // Note: Only useful for casting down class hierarchy
                        //       But it is also a sign of bad design.

Looking at the class.

class ObjMapData
{

There is no reason do your own memory management.

private:
char* itsMapData;

This should probably be a std::vector. Once you do that the rest becomes irelavant and all memory management issues are solved.

........
public:
ObjMapData();

Don't bother to test for NULL. Just delete.

~ObjMapData(){if(itsMapData!= NULL) delete[] itsMapData;}
ClearMemory() {if(itsMapData!= NULL) {delete[] itsMapData; itsMapData= NULL}}
.......

This obviously leaks the memory that you had before.

void SetMapData(char* ptrData) { itsMapData = ptrData;} // or should I use int*&ptrData??

Sure. But only if you need to expose that data.
I would rather encapsolate the manipulation of the data inside the class so you don't need to expose it.

char* GetMapData() const { return itsMapData;}
}

You don't show it so I have to assume you are not following the "Rule of Three" (look it up).

I would have done this:

class ObjMapData
{
    private:
        std::vector itsMapData;

    public:
        clearMemory() // lower case first letter of non type.
        {
            itsMapData.swap(std::vector());
        }
        void setMapData(std::vector& ptrData)
        {
             itsMapData.swap(ptrData);
        }
        char* getMapData() const { return &itsMapData[0];}
};

Addition based on comment:


(could be INT8, UINT8, INT16, UINT16, UINT32, INT32)

// Note the above data is not correctly aligned on all systems.

// INT8   INT8      INT16            INT16

01234567  01234567  0123456701234567 0123456701234567  Not a 32 Bit Boundary
|         |         |       |        |       |         |
8/16/32 Boundary    8/16 Boundary    8/16/32 Boundary  8/16 Boundary
          8 Boundary        8 Boundary       8 Boundary

But lets assume you just made a typo and you know what you are doing.

// Need room for
// INT 8 | INT 8 | INT 16 | INT 32 | INT 32
std::vector  data(96);
::read(fd, &data[0], 96);    // Wow look at that fits neatly.

INT8  d1 = *reinterpret_cast(&data[ 0]);  // The reinterpret cast is required
INT8  d2 = *reinterpret_cast(&data[ 8]);  // It also shows its really dangerious
INT16 d3 = *reinterpret_cast(&data[16]);
INT32 d4 = *reinterpret_cast(&data[32]);
INT32 d5 = *reinterpret_cast(&data[64]);

StackExchange Code Review Q#44901, answer score: 7