[Lldb-commits] [PATCH] D30054: Delete DataBufferMemoryMap

[Zachary Turner via lldb-commits]

I didn't refer to mmaping in the name because LLVM's MemoryBuffer is not
necessarily mmap'ed.  It might be mmap'ed and it might not be.  Depends on
various factors such as whether you specify the IsVolatile flag, how big
the file is, and maybe a few other things.

After this change we have DataBufferLLVM and DataBufferHeap.  But it turns
out an LLVM MemoryBuffer can also be backed by the heap, which now makes
DataBufferHeap redundant as well.  So I think longer term we might be able
to get rid of all of LLDB's DataBuffer stuff entirely, and everything will
just use llvm::MemoryBuffer directly.

What do you think?

On Mon, Feb 27, 2017 at 10:36 AM Jim Ingham <jingham at apple.com> wrote:

> This is kind of after the fact, but why didn't we reuse
> DataBufferMemoryMap for the Memory Map data buffer that now happens to be
> backed by an LLVM implementation?  DataBufferLLVM doesn't really tell
> anybody what the thing does w/o looking up the implementation.
>
> Jim
>
>
> > On Feb 27, 2017, at 2:56 AM, Pavel Labath via lldb-commits <
> lldb-commits at lists.llvm.org> wrote:
> >
> > I was thinking of a simple test like "call get on an existing file and
> > make sure it returns something reasonable" and "call get on a
> > non-existing file and make sure it returns null". This is a very thin
> > wrapper over over the llvm code, so I don't insist on it though...
> >
> > On 24 February 2017 at 15:18, Zachary Turner <zturner at google.com> wrote:
> >> I left out unit tests since we'd essentially be duplicating the unit
> tests
> >> of MemoryBuffer, and because it involves the file system (also this is
> >> temporary code until DataBuffer stuff goes away). Lmk if you disagree
> though
> >> On Fri, Feb 24, 2017 at 2:53 AM Pavel Labath via Phabricator
> >> <reviews at reviews.llvm.org> wrote:
> >>>
> >>> labath added a comment.
> >>>
> >>> I am not sure if this is a voting situation, but I agree with what
> Zachary
> >>> said above.
> >>>
> >>> Since we're already speaking about tests, it looks like the new
> >>> DataBufferLLVM class could use a unit test or two, just so we get in
> the
> >>> habit of writing those.
> >>>
> >>>
> >>> https://reviews.llvm.org/D30054
> >>>
> >>>
> >>>
> >>
> > _______________________________________________
> > lldb-commits mailing list
> > lldb-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20170227/7b6e4c69/attachment-0001.html>