Re: [Xen-devel] [PATCH 14/15] libxl: New API for providing OS events to libxl

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Ian Campbell writes ("Re: [Xen-devel] [PATCH 14/15] libxl: New API for 
providing OS events to libxl"):
> On Thu, 2011-12-08 at 19:53 +0000, Ian Jackson wrote:
> > All the existing files in libxl/ mention this nonexistent file
> > LICENCE.  I think we should fix this in a separate patch.  I'd argue
> > that my copying of the existing text isn't making the situation worse.
> 
> Sure, I didn't mean to suggest you needed to fix this in this series, I
> just happened to observe it here.

Right.  Yes.

> > > > +int libxl__ev_fd_register(libxl__gc *gc, libxl__ev_fd *ev,
> > > > +                          libxl__ev_fd_callback *func,
> > > > +                          int fd, short events) {
> > > 
> > > Strictly speaking CODING_STYLE requires the { to be on the next line.
> > 
> > Oh, I probably have a lot of those.  Damn.
> 
> Yeah, I refrained from commenting every time ;-)

Fixed.

> > You mean "int xs_path_is_subpath_p(const char *parent, const char *child)" ?
...
> Wouldn't most users of libxenstore doing watches need something like
> this (and probably either open code it or erroneously omit it)?

Many.  If you are careful enough with your tokens you might not need
to.

> Regardless of where it goes moving that logic into a helper function
> will make it clearer what is going on, both by having a descriptive name
> and allowing the logic to be a bit more spaced out / commented, the last
> clause in particular is slightly subtle.

OK, I have split this out into a function in libxenstore (in a
separate patch):

  /* Returns true if child is either equal to parent, or a node underneath
   * parent; or false otherwise.  Done by string comparison, so relative and
   * absolute pathnames never in a parent/child relationship by this
   * definition.  Cannot fail.
   */
  bool xs_path_is_subpath(const char *parent, const char *child);

You're right, the resulting code is clearer I think.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel