[llvm] r211184 - Fix a memory leak in the error path.

Rafael EspĂ­ndola rafael.espindola at gmail.com
Wed Jun 18 21:15:32 PDT 2014


> Incidentally, I noticed this while trying out a patch to cleanup
> another use of a isOwned flag, so I agree it is problematic :-(

A bot convinced me that this needed fixing sooner rather than later :-(

The general issue is that we normally want the convenience of the
buffer being destroyed by the thing that is using it (materializer,
ObjectFile), but sometimes we want to reuse the buffer.

The solution that is currently implemented is mostly:

* The thing using the buffer has a Boolean member saying if it owns it or not.
* The createThing methods take an equivalent Boolean argument.

The end result is somewhat brittle and fairly non idiomatic. A more
C++11ish solution would be for the users to have a
std::unique_ptr<MemoryBuffer> and a way of releasing ownership when
requested. That way the user buffer can "lend" the buffer and take it
back afterwards.

The attached patch implements it and IMHO is an improvement,  but it
is not without its ugly corner cases:

* The createThing methods now must not delete the buffer if they fail
to construct a Thing. This makes error handling on the common case
(Thing owns buffer) a bit more cumbersome.
* We need to add a way to release a buffer to the GVMaterializer
interface. This is a really odd single implementation interface and
needs a fairly thorough refactoring, so I am not too worried about it.
* materializeAllPermanently needs to know if it should release the
buffer before destroying the materializer.

Cheers,
Rafael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: t.patch
Type: text/x-patch
Size: 37076 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140619/ba655304/attachment.bin>


More information about the llvm-commits mailing list