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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Fri, 6 Jan 2012, Ian Jackson wrote:
> +struct libxl__egc {
> +    /* for event-generating functions only */
> +    struct libxl__gc gc;
> +};

[...]

>  /*
> + * Calling context and GC for event-generating functions:
> + *
> + * These are for use by parts of libxl which directly or indirectly
> + * call libxl__event_occurred.  These contain a gc but also a list of
> + * deferred events.
> + *
> + * Most code in libxlshould not need to initialise their own egc.
> + * Even functions which generate specific kinds of events don't need
> + * to - rather, they will be passed an egc into their own callback
> + * function and should just use the one they're given.
> + *
> + * A handy idiom for functions taking an egc is:
> + *     libxl__gc *gc = &egc->gc;
> + *
> + * Functions using LIBXL__INIT_EGC may *not* generally be called from
> + * within libxl, because libxl__egc_cleanup may call back into the
> + * application.  This should be documented near the function
> + * prototype(s) for callers of LIBXL__INIT_EGC and EGC_INIT.
> + *
> + * For the same reason libxl__egc_cleanup (or EGC_FREE) must be called
> + * with the ctx *unlocked*.  So the right pattern has the EGC_...
> + * macro calls on the outside of the CTX_... ones.
> + */
> +
> +/* egc initialisation and destruction: */
> +
> +#define LIBXL_INIT_EGC(egc,ctx) do{             \
> +        LIBXL_INIT_GC((egc).gc,ctx);            \
> +        /* list of occurred events tbd */       \
> +    } while(0)
> +
> +_hidden void libxl__egc_cleanup(libxl__egc *egc);
> +
> +/* convenience macros: */
> +
> +#define EGC_INIT(ctx)                       \
> +    libxl__egc egc[1]; LIBXL_INIT_EGC(egc[0],ctx);      \
> +    libxl__gc *gc = &egc->gc
> +
> +#define EGC_FREE           libxl__egc_cleanup(egc)

I still prefer the approach prototyped in
http://marc.info/?l=xen-devel&m=132371797519877: avoid introducing
restrictions on how libxl functions have to be called, unify egc and gc,
make it possible to take the lock recursively and use the nested counter
to figure out when to call the callbacks.
It would make my head hurt less when I have to read/write this code,
however others might react differently.

I don't think we'll ever get an agreement on this so I'll just step back
and let other people decide what is the right approach.

Only one more thing: I would kindly ask to move all these event related
functions to a different source file, to make it easier for people to
understand which ones are different from the rest of the library.

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