Getting an error using malloc, could be caused due previous possible mistake

78 Views Asked by At

I'm trying to make custom array class.

template<typename T>
class Array{
public:
    Array(size_t size) : array_size(size){
        this->m_Data  = (T*)malloc(sizeof(T) * size);
    }
    
    void insert(size_t index, T value){
        size_t temp_size = this->array_size + 1;
        T *temp = (T*)malloc(sizeof(T) * temp_size );
        //memcpy(temp, this->m_Data, this->array_size );
        for(size_t l = 0; l < temp_size; l++ )
            temp[l] = this->m_Data[l];

        for(size_t i = temp_size; i >= index; i--){
            temp[i] = temp[(i-1)];
        }
        temp[index - 1] = value;
        
        free(this->m_Data);
        this->m_Data = NULL;
        this->m_Data = temp;
        this->array_size = temp_size;
        //free(temp);
        //delete(temp);
    }
    
    void append(T value){
        size_t temp_size = this->array_size + 1;
        std::cout << "temp_size: " << temp_size << std::endl;
        std::cout << "sizeof(T): " << sizeof(T) << std::endl; // returns 4
        std::cout << "temp_size * sizeof(T): " << (temp_size * sizeof(T)) << std::endl; // returns 44
        T *temp = (T*)malloc((temp_size * sizeof(T)));     // error!
    }
    
    
private:
    size_t array_size;
    T *m_Data;
};

If I create an array with size smaller than 10, the malloc() function arises an error.

int main(int argc, char *argv[])
{
    Array<int> data(9); 
    data.insert(1, 100);  // seams to work fine
    data.append(100000);
}

If I use free(temp) in the insert() function, some values in the array are junk, but it seams logic to use: the error in the append() function might be related to this. Otherwise, the insert() function works fine.

T *temp = (T*)malloc((temp_size * sizeof(T))) in the append() function is basically the same as in the insert() function.

How come?

1

There are 1 best solutions below

3
PaulMcKenzie On BEST ANSWER

One error is that this results in a buffer overwrite, since temp_size is the number of items you allocated, thus temp[temp_size] is not valid:

T *temp = (T*)malloc(sizeof(T) * temp_size );
//...
for(size_t i = temp_size; i >= index; i--)
{
    temp[i] = temp[(i-1)]; // <-- Overwrite on first iteration

This probably should be:

for(size_t i = temp_size - 1; i >= index; i--)
{
    temp[i] = temp[(i-1)]; 

But there are other errors that can be easily triggered by using your code.

For example, Array<std::string> or simply Array<any_non_trivial_object> will cause an issue due to the usage of malloc and free instead of new[] and delete[].