RE: [PATCH 1/5] btrace: prepare for throwing exceptions when enabling btrace

["Metzger, Markus T" <markus dot t dot metzger at intel dot com>]

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: 25 January 2018 15:47
> To: Metzger, Markus T <markus.t.metzger@intel.com>; gdb-
> patches@sourceware.org
> Subject: Re: [PATCH 1/5] btrace: prepare for throwing exceptions when enabling
> btrace

Hello Pedro,

Thanks for your comments.

> > +  scoped_mmap () noexcept : m_mem (nullptr), m_length (0) {}
> > +  scoped_mmap (void *addr, size_t length, int prot, int flags, int fd,
> > +	       off_t offset) noexcept : m_mem (nullptr), m_length (length)
> > +    {
> > +      m_mem = mmap (addr, m_length, prot, flags, fd, offset);
> 
> If scoped_mmap is templated on object_type, why do we need the length
> parameter, in bytes?  Or conversely, if the interface is in terms of bytes, why do we
> need the template parameter?
> (AFAICS, it's for operator->, but, is that really useful compared to a mismatch in the
> API?  Seems like a layering violation to me.)

We need to store the length for the munmap().

The isn't really a template in the sense that it maps/unmaps an object.  It maps
some kernel-managed buffer and there happens to be a user-page at the front
that is described by this struct.  I made it a template to save casts when accessing
the user-page in the buffer.

I see how this can be confusing, though, so I'll change it to have this class just take
care about mapping and unmapping memory.


> You should add describing comments to the methods too.

Hmmm, given that they are both inline and trivial I wouldn't really know what to
write that isn't completely obvious.


> These seem like general enough utilities that might be better for the long run to put
> them straight in gdb/common/.  (Unit tests would be nice.)

I moved them into gdb/common/ but have not tried to make them any more generic.

I ran unit tests using "maint selftest".  Are they included in "make check", as well?

Thanks,
Markus.

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928