ravitheja added a comment. In https://reviews.llvm.org/D33674#790653, @labath wrote:
> 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. Ok I will write it like that. Please tell me if u want more changes, I will do them all together for the next 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)}; + ---------------- labath wrote: > 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? Ok I will use that. https://reviews.llvm.org/D33674 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
