[patch] Don't own the buffer in object::Binary

Rafael Espíndola rafael.espindola at gmail.com
Tue Aug 12 17:08:43 PDT 2014


ping. A rebased patch is attached.

On 6 August 2014 18:00, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
> An updated version is attached.
>
> The main difference is that now Object keeps a reference both to the
> data and to the name instead of copying the name to a std::string. I
> also changed Archive to provide MemoryBufferRef to access its members
> instead of MemoryBuffers.
>
> I think this is in good enough shape for a code review now :-)
>
>
> On 1 August 2014 16:32, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
>> There were quiet a few requests for not owning the buffer in
>> object::Binary, this an attempt to make that happen (and make it
>> obvious in the API).
>>
>> The patch is intentionally focused on lib/Object. If some other
>> structure had a std::unique_ptr<Binary>, now it also has a
>> std::unique_ptr<MemoryBuffer>.
>>
>> The main things It would be nice to have some feedback on:
>>
>> * MemoryBufferRef. Should this have another name? Should it implement
>> more of the MemoryBuffer API instead?
>>
>> * OwningBinary: We need something like this for the convenience
>> methods that construct an ObjectFile from a file name.
>>
>> * What should we do about the C API? Should it start wrapping
>> OwningBinary instead of Binary to keep the existing semantics? Should
>> we put a big warning in the 3.6 release and change it (I don't expect
>> too many users at this point)?
>>
>> * There is still a fixme about removing getMemoryBuffer from Archive.
>> I should have an updated patch with that early next weak, but I don't
>> expect it to change anything in a fundamental way.
>>
>> I am still running the tests with Valgrind, but we can probably start
>> the review in parallel.
>>
>> Cheers,
>> Rafael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: t.patch
Type: text/x-patch
Size: 66254 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140812/6e542270/attachment.bin>


More information about the llvm-commits mailing list