[Spice-devel] [PATCH spice-server 2/3] Add documentation for Dispatcher

Frediano Ziglio fziglio at redhat.com
Mon Sep 4 10:20:29 UTC 2017


> 
> ---
>  server/dispatcher.h | 113
>  ++++++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 92 insertions(+), 21 deletions(-)
> 
> diff --git a/server/dispatcher.h b/server/dispatcher.h
> index 97b01de9c..cc83a8fdc 100644
> --- a/server/dispatcher.h
> +++ b/server/dispatcher.h
> @@ -35,6 +35,17 @@ typedef struct Dispatcher Dispatcher;
>  typedef struct DispatcherClass DispatcherClass;
>  typedef struct DispatcherPrivate DispatcherPrivate;
>  
> +/* A Dispatcher provides inter-thread communication by serializing messages
> + * using a socketpair.
> + *

I would avoid to put implementation detail in the header unless this
possibly provide some API feature. In this case the user of the Dispatcher
should not care about how they are passed. Would be different for instance
if the dispatcher could move file descriptors between processes so this
would require unix sockets. But even so if this class should be moved
in Windows would be probably using a named pipe (which is completely
different from the unix ones) or DuplicateHandle.

> + * Message types are identified by a unique integer value must first by
> + * registered with the class (see dispatcher_register_handler()) before they
> + * can be sent. Sending threads can send a message using the
> + * dispatcher_send_message() function. The recieving thread can monitor the

typo, "recieving", there's another below

> + * dispatcher's 'receive' file descriptor (see dispatcher_get_recv_fd()) for
> + * activity and should call dispatcher_handle_recv_read() to process
> incoming
> + * messages.
> + */
>  struct Dispatcher
>  {
>      GObject parent;
> @@ -49,23 +60,50 @@ struct DispatcherClass
>  
>  GType dispatcher_get_type(void) G_GNUC_CONST;
>  
> +/* dispatcher_new
> + *
> + * Create a new Dispatcher object
> + *
> + * @max_message_type:   indicates the number of unique message types that
> can
> + *                      be handled by this dispatcher. Each message type is
> + *                      identified by an integer value between 0 and
> + *                      max_message_type-1.
> + * @opaque:             an arbitrary pointer that will be passed as the
> first
> + *                      argument to any registered handler functions
> + */
>  Dispatcher *dispatcher_new(size_t max_message_type, void *opaque);
>  

OT: I think max_message_type should be uint32_t, is always copied
in a uint32_t.

>  
> +/* The function signature for handlers of a specific message type */
>  typedef void (*dispatcher_handle_message)(void *opaque,
>                                            void *payload);
>  
> +/* The signature for a function that handles all messages (see
> + * dispatcher_register_universal_handler()) */
>  typedef void (*dispatcher_handle_any_message)(void *opaque,
>                                                uint32_t message_type,
>                                                void *payload);
>  
> +/* The signature of a function that handles async done notifcations for all
> + * messages that are registered with DISPATCHER_ASYNC ack type. See
> + * dispatcher_register_async_done_callback() and
> dispatcher_register_handler()
> + * for more information. */
>  typedef void (*dispatcher_handle_async_done)(void *opaque,
>                                               uint32_t message_type,
>                                               void *payload);
>  
>  
> -/*
> - * dispatcher_send_message
> +/* dispatcher_send_message
> + *
> + * Sends a message to the recieving thread over a socketpair. The message
> type

typo, also the socketpair

> + * must have been registered first (see dispatcher_register_handler()).
> + * @payload must be a buffer of the same size as the size registered for
> + * @message_type
> + *
> + * If the sent message is a message type that was registered with an ack
> type
> + * of DISPATCHER_ACK, this function will block until it receives an ACK from
> + * the receiving thread.
> + *
>   * @message_type: message type
>   * @payload:      payload
>   */
> @@ -78,25 +116,36 @@ enum {
>      DISPATCHER_ASYNC
>  };
>  
> -/*
> - * dispatcher_register_handler
> +/* dispatcher_register_handler
> + *
> + * This function registers a message type with the dispatcher, and registers
> + * @handler as the function that will handle incoming messages of this type.
> + * Based on the value of @ack, the dispatcher may also take additional
> actions
> + * after the message has been passed to the handler. You can only register a
> + * given message type once. For example, you cannot register two different
> + * handlers for the same message type with different @ack values.
> + *
>   * @dispatcher:     dispatcher
>   * @messsage_type:  message type
>   * @handler:        message handler
>   * @size:           message size. Each type has a fixed associated size.
>   * @ack:            One of DISPATCHER_NONE, DISPATCHER_ACK,
>   DISPATCHER_ASYNC.
> - *                  DISPATCHER_NONE - only send the message
> - *                  DISPATCHER_ACK - send an ack after the message
> - *                  DISPATCHER_ASYNC - call send an ack. This is per message
> type - you can't send the
> - *                  same message type with and without. Register two
> different
> - *                  messages if that is what you want.
> + *                  DISPATCHER_NONE - only call the message handler
> + *                  DISPATCHER_ACK - call the message handler and then send
> an ACK response to the sender
> + *                  DISPATCHER_ASYNC - call the message handler and then
> call the registered async_done handler

I still think the flag description is confusing.
Is not clear the subject. It change the communication, the description seems
to apply to the message receiver however the behaviour changes also for
the sender (in case of ACK will wait).

>   */
>  void dispatcher_register_handler(Dispatcher *dispatcher, uint32_t
>  message_type,
>                                   dispatcher_handle_message handler, size_t
>                                   size,
>                                   int ack);
>  
> -/*
> - * dispatcher_register_async_done_callback
> +/* dispatcher_register_async_done_callback
> + *
> + * register an async_done callback for this dispatcher. For all message
> types
> + * that were registered with a DISPATCHER_ASYNC ack type, this function will
> be
> + * called after the normal message handler, and the message will be passed
> to
> + * this function. Note that this function will execute within the receiving
> + * thread.
> + *
>   * @dispatcher:     dispatcher
>   * @handler:        callback on the receiver side called *after* the
>   *                  message callback in case ack == DISPATCHER_ASYNC.
> @@ -105,32 +154,54 @@ void dispatcher_register_async_done_callback(
>                                      Dispatcher *dispatcher,
>                                      dispatcher_handle_async_done handler);
>  
> -/*
> - * Hack to allow red_record to see the message being sent so it can record
> - * it to file.
> +/* dispatcher_register_universal_handler
> + *
> + * Register a universal handler that will be called when *any* message is
> + * recieved by the dispatcher. Only one universal handler can be registered.
> + * This feature is currently used to record all messages to a file for
> replay
> + * and debugging.

recieved typo.

Is not clear that this handler is called in any case and the normal handler
have to be registered and will be called too.

> + *
> + * @dispatcher:     dispatcher
> + * @handler:        a handler function
>   */
>  void dispatcher_register_universal_handler(Dispatcher *dispatcher,
>                                      dispatcher_handle_any_message handler);
>  
> -/*
> - *  dispatcher_handle_recv_read
> - *  @dispatcher: Dispatcher instance
> +/* dispatcher_handle_recv_read
> + *
> + * A convenience function that is intended to be called by the receiving
> thread
> + * to handle all incoming messages and execute any handlers for those
> messages.
> + * This function will handle all incoming messages until there is no more
> data
> + * to read, so multiple handlers may be executed froma  single call to

froma typo

> + * dispatcher_handle_recv_read().
> + *
> + * @dispatcher: Dispatcher instance
>   */
>  void dispatcher_handle_recv_read(Dispatcher *);
>  
> -/*
> - *  dispatcher_get_recv_fd
> +/* dispatcher_get_recv_fd
> + *
> + * This function returns the fd that is used by the receiving thread to
> listen
> + * for incoming messages. When the receiving thread detects incoming

is the last sentence finished ?

> + *
>   *  @return: receive file descriptor of the dispatcher
>   */
>  int dispatcher_get_recv_fd(Dispatcher *);
>  
> -/*
> - * dispatcher_set_opaque
> +/* dispatcher_set_opaque
> + *
> + * This 'opaque' pointer is user-defined data that will be passed as the
> first
> + * argument to all handler functions.
> + *
>   * @dispatcher: Dispatcher instance
>   * @opaque: opaque to use for callbacks
>   */
>  void dispatcher_set_opaque(Dispatcher *dispatcher, void *opaque);
>  
> +/* dispatcher_get_thread_id
> + *
> + * Returns the id of the thread that created this Dispatcher object
> + */
>  pthread_t dispatcher_get_thread_id(Dispatcher *self);
>  
>  #endif /* DISPATCHER_H_ */

Thanks for the documentation, we really need a lot of it.

Frediano


More information about the Spice-devel mailing list