C++ malloc/realloc weird behavior

Question

I was programming a dynamic array for my own use, that i wanted pre-set with zeros.

template <class T>
dynArr<T>::dynArr()
{
rawData = malloc(sizeof(T) * 20); //we allocate space for 20 elems
memset(this->rawData, 0, sizeof(T) * 20); //we zero it!
currentSize = 20;
dataPtr = static_cast<T*>(rawData); //we cast pointer to required datatype.
}

And this part works - iterating by loop with dereferencind the dataPtr works great. Zeros.

Yet, reallocation behaves (in my opinion) at least a bit strange. First you have to look at reallocation code:

template <class T>
void dynArr<T>::insert(const int index, const T& data)
{

    if (index < currentSize - 1)
    {
        dataPtr[index] = data; //we can just insert things, array is zero-d
    }

    else
    {
        //TODO we should increase size exponentially, not just to the element we want

        const size_t lastSize = currentSize; //store current size (before realloc). this is count not bytes.

        rawData = realloc(rawData, index + 1); //rawData points now to new location in the memory
        dataPtr = (T*)rawData;
        memset(dataPtr + lastSize - 1, 0, sizeof(T) * index - lastSize - 1); //we zero from ptr+last size to index

        dataPtr[index] = data;
        currentSize = index + 1;
    }

}

Simple, we realloc data up to index+1, and set yet-non-zeroed memory to 0.

As for a test, i first inserted 5 on position 5 on this array. Expected thing happened - 0,0,0,0,5,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0

Yet, inserting something else, like insert(30,30) gives me strange behavior:

0, 0, 0, 0, 0, 5, 0, -50331648, 16645629, 0, 523809160, 57600, 50928864, 50922840, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 30,

What the hell, am i not understanding something here? shouldnt realloc take all the 20 previously set memory bytes into account? What sorcery is going on here.

Answer 1

Problem 1:

You are using the wrong size in the call to realloc. Change it to:

rawData = realloc(rawData, sizeof(T)*(index + 1)); 

If rawData is of type T*, prefer

rawData = realloc(rawData, sizeof(*rawData)*(index + 1)); 

Problem 2:

The last term of the following is not right.

memset(dataPtr + lastSize - 1, 0, sizeof(T) * index - lastSize - 1); 

You need to use:

memset(dataPtr + lastSize - 1, 0, sizeof(T) * (index - lastSize - 1));
                               //  ^^              ^^
                               // size      *  The number of objects 

Problem 3:

Assigning to dataPtr using

dataPtr[index] = data;

is a problem when memory is obtained using malloc or realloc. malloc family of functions return just raw memory. They don't initialize objects. Assigning to uninitialized objects is a problem for all non-POD types.

Problem 4:

If T is type with virtual member functions, using memset to zero out memory will most likely lead to problems.

---

Suggestion for fixing all the problems:

It will be much better to use new and delete since you are in C++ land.

template <class T>
dynArr<T>::dynArr()
{
   currentSize = 20;
   dataPtr = new T[currentSize];
   // Not sure why you need rawData
}

template <class T>
void dynArr<T>::insert(const int index, const T& data)
{
   if (index < currentSize - 1)
   {
      dataPtr[index] = data;
   }

   else
   {
      const size_t lastSize = currentSize;
      T* newData = new T[index+1];
      std::copy(dataPtr, dataPtr+lastSize, newData);
      delete [] dataPtr;
      dataPtr = newData;
      dataPtr[index] = data;
      currentSize = index + 1;
   }
}

Please note that the suggested change will work only if T is default constructible.

This will also take care of the problems 3 and 4 outlined above.