labath added inline comments.
================
Comment at: source/Plugins/Process/Linux/ProcessorTrace.cpp:307
+
+Status ProcessorTraceMonitor::Destroy() {
+ Status error;
----------------
ravitheja wrote:
> labath wrote:
> > I'd like this work to be done in the destructor. Just like nobody should
> > see a constructed-but-not-yet-initialized object, so the
> > destroyed-but-not-destructed state should not exist. Then you don't need to
> > worry about setting the state of the object to some sane values.
> >
> > In fact, if you utilize std::unique_ptr capabilities, then you may not even
> > need a destructor at all. Just define a class
> > ```
> > class munmap_delete {
> > size_t m_length;
> >
> > public:
> > munmap_delete(size_t length) : m_length(length) {}
> > void operator()(void *ptr) { munmap(ptr, m_length); }
> > };
> > ```
> >
> > and then you can just declare `
> > `std::unique_ptr<perf_event_mmap_page, munmap_delete>
> > mmap_base(mmap_result, munmap_delete(mmap_size))`
> >
> > This way, the allocation and deallocation code are close together, and you
> > don't need to write yourself `// we allocated extra page` reminders.
> >
> > Since we store m_aux in a ArrayRef already, this would require duplicating
> > the pointer in the unique_ptr member. I am fine with that, but if you want
> > to avoid it you can just have a getAux() member function which sythesizes
> > the arrayref on demand.
> I can get the desired effect, by moving this function to the destructor, but
> we won't be able to catch munmap errors. I guess logging should be fine ?
> right now we were able to report errors in munmap.
They only way I see that munmap can fail is if you pass it a garbage argument.
```
On success, munmap() returns 0, on failure -1, and errno is set (probably to
EINVAL). :D
```
That is a programmer error, so I would be fine with even asserting on it, but
logging and ignoring is fine as well. It's not like you can do anything better
at that point anyway -- you're already ignoring the return value of Destroy...
https://reviews.llvm.org/D33674
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
