[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] PATCH: Pass a callback for freeing opaque data when registering handle/timer events



On Mon, Nov 17, 2008 at 05:02:40PM +0000, Daniel P. Berrange wrote:
> When registering for a file descriptor event, or timer events, the event
> callback has an associated 'void *opaque' data blob. When removing a
> registered event, the removal may be done asynchronously to allow safe
> removal from within a callback. This means that it is not safe for the
> application to assume they can free the 'void *opaque' data immediately
> after calling virEventRemoveHandle/Timer. So, we extend the AddHandle/Timer
> method to allow a 2nd callback to be provided. This callback is used to
> free the 'void *opaque' data at the appropriate (safe) point in time.

  okay, it makes more sense to provide the API framework on how to free
the data

> diff --git a/include/libvirt/libvirt.h b/include/libvirt/libvirt.h
> --- a/include/libvirt/libvirt.h
> +++ b/include/libvirt/libvirt.h
> @@ -1132,21 +1132,39 @@ typedef void (*virEventHandleCallback)(i
>  typedef void (*virEventHandleCallback)(int watch, int fd, int events, void *opaque);
>  
>  /**
> + * virEventHandleFreeFunc:
> + *
> + * @opaque: user data to be free'd
> + *
> + * Callback invoked when memory associated with a file handle
> + * watch is being freed. Will be passed a pointer to the  application's
> + * opaque data blob.
> + */
> +typedef void (*virEventHandleFreeFunc)(void *opaque);
> +
> +/**
>   * virEventAddHandleFunc:
>   * @fd: file descriptor to listen on
>   * @event: bitset of events on which to fire the callback
> - * @cb: the callback to be called
> + * @cb: the callback to be called when an event occurrs
> + * @ff: the callback invoked to free opaque data blob
>   * @opaque: user data to pass to the callback
>   *
>   * Part of the EventImpl, this callback Adds a file handle callback to
>   * listen for specific events. The same file handle can be registered
>   * multiple times provided the requested event sets are non-overlapping
>   *
> + * If the opaque user data requires free'ing when the handle
> + * is unregistered, then a 2nd callback can be supplied for
> + * this purpose.
> + *
>   * Returns a handle watch number to be used for updating
>   * and unregistering for events
>   */
>  typedef int (*virEventAddHandleFunc)(int fd, int event,
> -                                     virEventHandleCallback cb, void *opaque);
> +                                     virEventHandleCallback cb,
> +                                     virEventHandleFreeFunc ff,
> +                                     void *opaque);
>  
>  /**
>   * virEventUpdateHandleFunc:
> @@ -1163,7 +1181,11 @@ typedef void (*virEventUpdateHandleFunc)
>   * @watch: file descriptor watch to stop listening on
>   *
>   * Part of the EventImpl, this user-provided callback is notified when
> - * an fd is no longer being listened on
> + * an fd is no longer being listened on.
> + *
> + * If a virEventHandleFreeFunc was supplied when the handle was
> + * registered, it will be invoked some time during, or after this 
> + * function call, when it is safe to release the user data.
>   */
>  typedef int (*virEventRemoveHandleFunc)(int watch);
>  
> @@ -1178,17 +1200,35 @@ typedef void (*virEventTimeoutCallback)(
>  typedef void (*virEventTimeoutCallback)(int timer, void *opaque);
>  
>  /**
> + * virEventTimeoutFreeFunc:
> + *
> + * @opaque: user data to be free'd
> + *
> + * Callback invoked when memory associated with a timer 
> + * is being freed. Will be passed a pointer to the  application's
> + * opaque data blob.
> + */
> +typedef void (*virEventTimeoutFreeFunc)(void *opaque);
> +
> +/**
>   * virEventAddTimeoutFunc:
>   * @timeout: The timeout to monitor
>   * @cb: the callback to call when timeout has expired
> + * @ff: the callback invoked to free opaque data blob
>   * @opaque: user data to pass to the callback
>   *
>   * Part of the EventImpl, this user-defined callback handles adding an
>   * event timeout.
>   *
> + * If the opaque user data requires free'ing when the handle
> + * is unregistered, then a 2nd callback can be supplied for
> + * this purpose. 
> + *
>   * Returns a timer value
>   */
> -typedef int (*virEventAddTimeoutFunc)(int timeout, virEventTimeoutCallback cb,
> +typedef int (*virEventAddTimeoutFunc)(int timeout,
> +                                      virEventTimeoutCallback cb,
> +                                      virEventTimeoutFreeFunc ff,
>                                        void *opaque);
>  
>  /**
> @@ -1207,6 +1247,10 @@ typedef void (*virEventUpdateTimeoutFunc
>   *
>   * Part of the EventImpl, this user-defined callback removes a timer
>   *
> + * If a virEventTimeoutFreeFunc was supplied when the handle was
> + * registered, it will be invoked some time during, or after this 
> + * function call, when it is safe to release the user data.
> + *
>   * Returns 0 on success, -1 on failure
>   */
>  typedef int (*virEventRemoveTimeoutFunc)(int timer);

  Okay, the API change looks fine, no problem for me.


  and everything else seems to be a direct set of change related to the
extra callback function,

  +1

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]