xcsp3team / XCSP3-CPP-Parser

XCSP3 Core Parser in C++
MIT License
19 stars 11 forks source link

Potential memory leak #22

Closed massimomorara closed 4 years ago

massimomorara commented 4 years ago

Again regarding the parse(istream &in) method in "src/XCSP3CoreParser.cc".

In this method, you directly manage dynamic memory, allocating a buffer

char *buffer = new char[bufSize];

and deleting it at the end

delete[] buffer;

Problem: if, parsing the file, an exception is thrown (maybe catched, but also rethrown) the buffer isn't deleted and you get a memory leak.

General C++ suggestion: avoid direct dynamic memory managment like the plague. Instead, use STL containers and, when you really have to allocate/deallocate memory, manage it through smart pointers.

Specific suggestion: transform buffer in a std::unique_ptr

std::unique_ptr<char[]> buffer { new char[bufSize] };

call the get() method when you need the pointer

// ...
in.read(buffer.get(), bufSize);
// ...
parserCtxt = xmlCreatePushParserCtxt(&handler, &cspParser, buffer.get(), size, filename);
// ...
in.read(buffer.get(), bufSize);
// ...
xmlParseChunk(parserCtxt, buffer.get(), size, 0);
// ...
xmlParseChunk(parserCtxt, buffer.get(), 0, 1);

and cancel the final delete[] buffer; instruction.

Using a std::unique_ptr, also in case of exception the destructor of buffer is called, the related memory is deallocated and the memory leak is avoided.

xcsp3team commented 4 years ago

Thanks again.