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

Rafael EspĂ­ndola rafael.espindola at gmail.com
Mon Jun 23 07:24:11 PDT 2014


ping.  A rebased patch is attached.



On 19 June 2014 00:15, Rafael EspĂ­ndola <rafael.espindola at gmail.com> wrote:
>> 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/20140623/3c9b9c6d/attachment.bin>


More information about the llvm-commits mailing list