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

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Ian Campbell writes ("Re: [Xen-devel] [PATCH 1/9] libxl: New API for providing 
OS events to libxl"):
> On Fri, 2012-01-13 at 19:25 +0000, Ian Jackson wrote:
> > +struct libxl__ev_fd {
> > +    /* caller should include this in their own struct */
> > +    /* read-only for caller, who may read only when registered: */
> > +    int fd;
> > +    short events;
> > +    libxl__ev_fd_callback *func;
> 
> Are there actually cases where a caller would want to read these?
> 
> The most obvious case would be in the callback but it already gets given
> all three there.
> 
> Not suggesting we disallow this I'm just curious.

This is a change from my previous version of this series.  When
writing the device removal code I found myself wanting to read the
path member of a libxl__ev_xswatch:

+static void devstate_timeout(libxl__egc *egc, libxl__ev_time *ev,
+                             const struct timeval *requested_abs)
+{
+    EGC_GC;
+    libxl__ev_devstate *ds = CONTAINER_OF(ev, *ds, timeout);
+    LIBXL__LOG(CTX, LIBXL__LOG_DEBUG, "backend %s wanted state %d "
+               " timed out", ds->watch.path, ds->wanted);
                              ^^^^^^^^^^^^^^
+    libxl__ev_devstate_cancel(gc, ds);
+    ds->callback(egc, ds, ERROR_TIMEDOUT);
+}

So my options were:
 0. Not print the path, rendering the message almost useless
 1. Copy the path an extra time, pointlessly
 2. Relax the rules about the contents of libxl__ev_xswatch, making
    a special exception for this particular struct
 3. Relax the rules about the contents of libxl__ev_* generally
 4. Change the API of libxl__ev_* so that the caller always writes
    the fd, path, etc.

Of these 3 seemed best.  I considered 4.; but the result would be that
libxl__ev_KIND_register wouldn't take the arguments specifying what to
wait for, and that seemed a step too far.  Particularly since needing
to read inside the struct isn't all that common.

> Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

Thanks,
Ian.

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