Re: [Xen-devel] [PATCH 11/11] libxl: -Wunused-parameter

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Ian Campbell writes ("Re: [PATCH 11/11] libxl: -Wunused-parameter"):
> I'm not entirely sure how I feel about this patch generally (it's quite
> a bit of mess, but the bugs it would have found are real). It's also
> quite a lot of churn for 4.2.

Yes.

> On the other hand we are likely to want to backport lots of libxl fixes
> for 4.2.1 (I was actually considering an exception to the "no new
> features" rule for 4.2.1 for xm parity causing patches) and having this
> in 4.2 would make that cleaner.

Indeed.

> I guess I come down (just) on the side of taking this, when it is baked.

OK.  Having slept on it I think overall this is an improvement and
will help us in the future.

> > @@ -700,6 +702,8 @@ int libxl_domain_remus_start(libxl_ctx *ctx, 
> > libxl_domain_remus_info *info,
> >      libxl__domain_suspend_state *dss;
> >      int rc;
> > 
> > +    USE(recv_fd); /* TODO get rid of this and actually use it! */
> 
> You've only just introduced TODO REMUS...

Point.

> > @@ -1019,6 +1019,7 @@ void libxl_osevent_occurred_fd(libxl_ctx *ctx, void 
> > *for_libxl,
> > 
> >      assert(fd == ev->fd);
> >      revents &= ev->events;
> > +    USE(events); /* we use our own idea of what we asked for */
> 
> What is the point of this argument then?

It's there for consistency with poll(2)'s API.

> Is getting an event we weren't expecting a log-worthy occurrence?

events might be different from those we requested because events might
be "in flight" from the application's call to poll, to us, while we
register/deregister them.  So this is not a logworthy event.  I guess
this property of the registration API is not documented and should be
(although it amounts only to a relaxation from the point of view of
the application).

Also I found a mistake in a related comment so I will fix these
comments in another patch.

> > diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
> > index 9c92ae6..a094965 100644
> > --- a/tools/libxl/libxl_pci.c
> > +++ b/tools/libxl/libxl_pci.c
> > @@ -800,6 +800,10 @@ static int pci_ins_check(libxl__gc *gc, uint32_t 
> > domid, const char *state, void
> >  {
> >      char *orig_state = priv;
> > 
> > +    USE(gc);
> > +    USE(domid);
> > +    USE(priv);
> 
> You actually use priv above.

Fixed.

Ian.

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