labath added a comment.
In https://reviews.llvm.org/D33674#790595, @ravitheja wrote:
> Yes you can start looking at it. The thing with munmap stuff is the
> following, you basically don't want to give the user an opportunity to have
> an uninitialized or instances which have been destroyed but not deleted.
> So I removed the Destroy function from the public space. Now the user need
> not worry about Destroy function.
Ok, so that was one of my reasons for doing this. The other one is internal
code cleanlyness -- it's much easier to verify that the code is healthy by just
looking at it when the initialization and deinitialization is close together.
unique_ptr allows you to do that. In this code
if ((result = mmap(..., size)) != MAP_FAILED)
ptr_up = std::unique_ptr(result, mmap_deleter(size));
the init and deinit code is on adjecant line, and it's guaranteed than memory
will be freed. Here:
first_function() {
ptr = mmap(..., size);
ref=ArrayRef(ptr, size-1);
...
}
...
second_function() {
...
// Remember to increment size by one
munmap(ref.data(), ref.size()+1);
...
}
it's not so obvious because the code is far apart and you need to carry state
around. To verify correctness I need to look at the two pieces of code, and
then find all possible code paths between them.
> Regarding the testing strategy, we have tests at two levels, one at the SB
> API level and the other at the tool level.
Cool, are you going to put some of those tests in this patch?
================
Comment at: source/Plugins/Process/Linux/ProcessorTrace.cpp:393
+ llvm::SmallVector<MutableArrayRef<uint8_t>, 2> parts = {
+ src.slice(src_cyc_index), src.drop_back(src.size() - src_cyc_index)};
+
----------------
ravitheja wrote:
> labath wrote:
> > Isn't the drop-back expression equivalent to
> > `src.take_front(src_cyc_index)`?
> The problem with that is I don't see the documentation of some functions and
> I have to dig in the code.
> Which is why I did not used it.
That's fine, there are a lot of these APIs and it's not possible to remember
all of them. However, now that you know about the function, wouldn't you agree
that it's cleaner to use it?
https://reviews.llvm.org/D33674
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
