It looks like that in this case (and possibly many others) a wrapper
class is not necessary. If we just want to log the inputs and outputs
of mmap(), we just need to wrap the factory function (as we have done
here) and let the users access the llvm class directly.
On 27 February 2017 at 20:05, Jim
Having library functions that don't return good errors seems like such an
obvious failing that it shouldn't be hard to motivate fixing that. Then our
logging can go in the wrapper classes, using those errors. That seems like a
pattern that solves the "don't duplicate code" problem and the "lld
Oh that wasn't in response to the comment about wrapper classes, that was
just a general comment about the fact that we lose logging by moving to
LLVM's implementation at all. If we have our own implementation, we could
in theory log at various stages of the mmaping process, but by moving to a
lib
> On Feb 27, 2017, at 11:49 AM, Zachary Turner wrote:
>
> There may be some cases where we're no longer mmaping where we used to, but
> looking at LLVM's implementation, that would only happen if the file is
> fairly small, and there's a comment in LLVM explaining why they don't mmap
> small
There may be some cases where we're no longer mmaping where we used to, but
looking at LLVM's implementation, that would only happen if the file is
fairly small, and there's a comment in LLVM explaining why they don't mmap
small files. So I think that's actually an improvement for LLDB (although
I
I worry about stripping out the wrappers, because there are some differences in
how lldb operates from llvm that I don't think we want to push down into llvm -
in this case I'm thinking particularly about logging. DataBufferMemoryMap did
a bunch of logging, which presumably would get lost if yo
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 DataBuff
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
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
I am going to move the constructor to private for now, this should
eliminate any disagreement about whether to add null checks, as it is now
literally impossible to construct one with a null pointer. For now this
doesn't matter since nobody is even using the MemoryBuffer constructor
except from in
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 <
revi...@revie
11 matches
Mail list logo
