Re: [Xen-devel] [PATCH 16/29] libxl: ao: Provide manip_refcnt

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Tue, 2015-03-31 at 19:23 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [Xen-devel] [PATCH 16/29] libxl: ao: Provide 
> manip_refcnt"):
> > On Tue, 2015-02-10 at 20:10 +0000, Ian Jackson wrote:
> > > +/*
> > > + * A "manip" is a libxl public function manipulating this ao, which
> > > + * has a pointer to it.  We have to not destroy it while that's the
> > > + * case, obviously.
> > 
> > It might be nice to the reader to make a reference to the "An ao and its
> > gc may be accessed only with the ctx lock held." sentence in the overall
> > ao docs?
> 
> This seems to have been documented already in May 2012 in exactly
> those terms, at the top of struct libxl__ao, in 738b3d442522.  Am I
> confused ?

Right, my suggestions was to make a reference to that comment. One might
expect ao__manip_enter to take care of locking for you as part of the
"prepare this thing to be manipulated".

We don't seem to have an ASSERT_CTX_LOCKED construct, or I would suggest
using it in cases like these.

> I have added a comment about locking in the manip functions' doc
> comment.
> 
> > [...]
> > > +        libxl__ao__destroy(ctx,ao);
> > 
> > Nit: missing space after ",".
> 
> Fixed.
> 
> > But other than those and including if you disagree about extending the
> > comment, since it's not a big deal:
> >         Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

This stands.

Ian.



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel