J. Doe J. Doe - 1 month ago 15
C++ Question

Can't detect memory leak in C++ code

I am trying to detect the memory leak error in my code in c++. I have tried using valgrind but it is not providing me enough info to debug my code. Any help is greatly appreciated. The code is extracted from a .h file I was working on to implement a data structure in C++11.

class key_value_sequences {
public:
// YOU SHOULD USE C++ CONTAINERS TO AVOID RAW POINTERS
// IF YOU DECIDE TO USE POINTERS, MAKE SURE THAT YOU MANAGE MEMORY PROPERLY

// IMPLEMENT ME: SHOULD RETURN SIZE OF A SEQUENCE FOR GIVEN KEY
// IF NO SEQUENCE EXISTS FOR A GIVEN KEY RETURN -1
int size(int key) const
{
for( int i=0; i<pairs.size(); i++)
{
if( pairs[i][0] == key )
{
return pairs[i].size() - 1;
}
}
return -1;
}

// IMPLEMENT ME: SHOULD RETURN POINTER TO A SEQUENCE FOR GIVEN KEY
// IF NO SEQUENCE EXISTS FOR A GIVEN KEY RETURN nullptr
const int* data(int key) const
{
int length = size(key);

if( length == -1 )
return NULL;

int * data;
data = new int[ length ];

for( int i=0; i<pairs.size(); i++)
{
if( pairs[i][0] == key )
{
for( int j=1; j<pairs[i].size(); j++)
{
data[j-1] = pairs[i][j];
}
}
}
return data;
}

// IMPLEMENT ME: INSERT VALUE INTO A SEQUENCE IDENTIFIED BY GIVEN KEY
void insert(int key, int value)
{
bool found = false;
int pos = -1;

for( int i=0; i<pairs.size(); i++)
{
if( pairs[i][0] == key )
{
found = true;
pos = i;
}
}

// insert if found in old vector,
// else make a new vector

if( found )
{
pairs[pos].push_back(value);
}
else
{
// insert new value set for the key
std::vector<int> v;
v.push_back(key);
v.push_back(value);
pairs.push_back(v);
}
}
private:

// list is stored as a vector and
std::vector< std::vector<int> > pairs;

}; // class key_value_sequences

Answer

This code is not enough to tell you what is happening, we can only make a guess.

I would say that your leak comes from the method data: you create a copy of the data and then return a pointer to this copy. It would be better to return an unique_ptr, so the memory is released automatically.

If you cannot change the definition of the method, then you could change the body of the method by this one:

int length = size(key);

if( length == -1 )
    return nullptr;

for( int i=0; i<pairs.size(); i++)
{
    if( pairs[i][0] == key )
    {
      return &pairs[i][1];
    }
}
return nullptr;