Re: [PATCH 02/14] RedCharDevice: Add typedef for class virtual functions

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

 



Hmm, I don't think I really agree here. I don't object to the typedefs, but in
general I'd prefer a wrapper function to a cast for added type-safety.

Reviewed-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>


On Thu, 2016-04-07 at 17:11 -0500, Jonathon Jongsma wrote:
> From: Christophe Fergeau <cfergeau@xxxxxxxxxx>
> 
> When initializing the class vfuncs, this allows us to cast already
> existing functions to the right type rather than having a wrapper
> function whose only purpose is to cast one argument to the right type.
> ---
>  server/char-device.h | 33 ++++++++++++++++++---------------
>  server/reds.c        | 16 ++--------------
>  server/smartcard.c   | 16 ++--------------
>  server/spicevmc.c    | 16 ++--------------
>  4 files changed, 24 insertions(+), 57 deletions(-)
> 
> diff --git a/server/char-device.h b/server/char-device.h
> index f4d6283..4c8928b 100644
> --- a/server/char-device.h
> +++ b/server/char-device.h
> @@ -47,6 +47,16 @@ struct SpiceCharDeviceState
>      RedCharDevicePrivate *priv;
>  };
>  
> +typedef RedCharDeviceMsgToClient*
> (*RedCharDeviceReadOneMsgFromDevice)(SpiceCharDeviceInstance *sin,
> +                                                                         void
> *opaque);
> +typedef RedCharDeviceMsgToClient*
> (*RedCharDeviceRefMsgToClient)(RedCharDeviceMsgToClient *msg,
> +                                                                   void
> *opaque);
> +typedef void (*RedCharDeviceUnrefMsgToClient)(RedCharDeviceMsgToClient *msg,
> void *opaque);
> +typedef void (*RedCharDeviceSendMsgToClient)(RedCharDeviceMsgToClient *msg,
> RedClient *client, void *opaque);
> +typedef void (*RedCharDeviceSendTokensToClient)(RedClient *client, uint32_t
> tokens, void *opaque);
> +typedef void (*RedCharDeviceOnFreeSelfToken)(void *opaque);
> +typedef void (*RedCharDeviceRemoveClient)(RedClient *client, void *opaque);
> +
>  struct RedCharDeviceClass
>  {
>      GObjectClass parent_class;
> @@ -58,28 +68,21 @@ struct RedCharDeviceClass
>  
>      /* reads from the device till reaching a msg that should be sent to the
> client,
>       * or till the reading fails */
> -    RedCharDeviceMsgToClient*
> (*read_one_msg_from_device)(SpiceCharDeviceInstance *sin,
> -                                                          void *opaque);
> -    RedCharDeviceMsgToClient* (*ref_msg_to_client)(RedCharDeviceMsgToClient
> *msg,
> -                                                   void *opaque);
> -    void (*unref_msg_to_client)(RedCharDeviceMsgToClient *msg,
> -                                void *opaque);
> -    void (*send_msg_to_client)(RedCharDeviceMsgToClient *msg,
> -                               RedClient *client,
> -                               void *opaque); /* after this call, the message
> is unreferenced */
> -
> +    RedCharDeviceReadOneMsgFromDevice read_one_msg_from_device;
> +    RedCharDeviceRefMsgToClient ref_msg_to_client;
> +    RedCharDeviceUnrefMsgToClient unref_msg_to_client;
> +    /* after this call, the message is unreferenced */
> +    RedCharDeviceSendMsgToClient send_msg_to_client;
>      /* The cb is called when a predefined number of write buffers were
> consumed by the
>       * device */
> -    void (*send_tokens_to_client)(RedClient *client, uint32_t tokens, void
> *opaque);
> -
> +    RedCharDeviceSendTokensToClient send_tokens_to_client;
>      /* The cb is called when a server (self) message that was addressed to
> the device,
>       * has been completely written to it */
> -    void (*on_free_self_token)(void *opaque);
> -
> +    RedCharDeviceOnFreeSelfToken on_free_self_token;
>      /* This cb is called if it is recommanded that a client will be removed
>       * due to slow flow or due to some other error.
>       * The called instance should disconnect the client, or at least the
> corresponding channel */
> -    void (*remove_client)(RedClient *client, void *opaque);
> +    RedCharDeviceRemoveClient remove_client;
>  };
>  
>  GType red_char_device_get_type(void) G_GNUC_CONST;
> diff --git a/server/reds.c b/server/reds.c
> index e3f479d..792e45f 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -859,18 +859,6 @@ static RedCharDeviceMsgToClient
> *vdi_port_read_one_msg_from_device(SpiceCharDevi
>      return NULL;
>  }
>  
> -static RedCharDeviceMsgToClient
> *vdi_port_ref_msg_to_client(RedCharDeviceMsgToClient *msg,
> -                                                            void *opaque)
> -{
> -    return vdi_port_read_buf_ref(msg);
> -}
> -
> -static void vdi_port_unref_msg_to_client(RedCharDeviceMsgToClient *msg,
> -                                         void *opaque)
> -{
> -    vdi_port_read_buf_unref(msg);
> -}
> -
>  /* after calling this, we unref the message, and the ref is in the instance
> side */
>  static void vdi_port_send_msg_to_client(RedCharDeviceMsgToClient *msg,
>                                          RedClient *client,
> @@ -4330,8 +4318,8 @@
> red_char_device_vdi_port_class_init(RedCharDeviceVDIPortClass *klass)
>      object_class->constructed = red_char_device_vdi_port_constructed;
>  
>      char_dev_class->read_one_msg_from_device =
> vdi_port_read_one_msg_from_device;
> -    char_dev_class->ref_msg_to_client = vdi_port_ref_msg_to_client;
> -    char_dev_class->unref_msg_to_client = vdi_port_unref_msg_to_client;
> +    char_dev_class->ref_msg_to_client =
> (RedCharDeviceRefMsgToClient)vdi_port_read_buf_ref;
> +    char_dev_class->unref_msg_to_client =
> (RedCharDeviceUnrefMsgToClient)vdi_port_read_buf_unref;
>      char_dev_class->send_msg_to_client = vdi_port_send_msg_to_client;
>      char_dev_class->send_tokens_to_client = vdi_port_send_tokens_to_client;
>      char_dev_class->remove_client = vdi_port_remove_client;
> diff --git a/server/smartcard.c b/server/smartcard.c
> index ba6f2f5..eb68a8b 100644
> --- a/server/smartcard.c
> +++ b/server/smartcard.c
> @@ -169,18 +169,6 @@ static RedCharDeviceMsgToClient
> *smartcard_read_msg_from_device(SpiceCharDeviceI
>      return NULL;
>  }
>  
> -static RedCharDeviceMsgToClient
> *smartcard_ref_msg_to_client(RedCharDeviceMsgToClient *msg,
> -                                                             void *opaque)
> -{
> -    return smartcard_ref_vsc_msg_item((MsgItem *)msg);
> -}
> -
> -static void smartcard_unref_msg_to_client(RedCharDeviceMsgToClient *msg,
> -                                          void *opaque)
> -{
> -    smartcard_unref_vsc_msg_item((MsgItem *)msg);
> -}
> -
>  static void smartcard_send_msg_to_client(RedCharDeviceMsgToClient *msg,
>                                           RedClient *client,
>                                           void *opaque)
> @@ -868,8 +856,8 @@
> red_char_device_smartcard_class_init(RedCharDeviceSmartcardClass *klass)
>      object_class->finalize = red_char_device_smartcard_finalize;
>  
>      char_dev_class->read_one_msg_from_device =
> smartcard_read_msg_from_device;
> -    char_dev_class->ref_msg_to_client = smartcard_ref_msg_to_client;
> -    char_dev_class->unref_msg_to_client = smartcard_unref_msg_to_client;
> +    char_dev_class->ref_msg_to_client =
> (RedCharDeviceRefMsgToClient)smartcard_ref_vsc_msg_item;
> +    char_dev_class->unref_msg_to_client =
> (RedCharDeviceUnrefMsgToClient)smartcard_unref_vsc_msg_item;
>      char_dev_class->send_msg_to_client = smartcard_send_msg_to_client;
>      char_dev_class->send_tokens_to_client = smartcard_send_tokens_to_client;
>      char_dev_class->remove_client = smartcard_remove_client;
> diff --git a/server/spicevmc.c b/server/spicevmc.c
> index 1c41845..b6ba19d 100644
> --- a/server/spicevmc.c
> +++ b/server/spicevmc.c
> @@ -119,18 +119,6 @@ static void spicevmc_pipe_item_unref(SpiceVmcPipeItem
> *item)
>      }
>  }
>  
> -static RedCharDeviceMsgToClient
> *spicevmc_chardev_ref_msg_to_client(RedCharDeviceMsgToClient *msg,
> -                                                                    void
> *opaque)
> -{
> -    return spicevmc_pipe_item_ref((SpiceVmcPipeItem *)msg);
> -}
> -
> -static void spicevmc_chardev_unref_msg_to_client(RedCharDeviceMsgToClient
> *msg,
> -                                                 void *opaque)
> -{
> -    spicevmc_pipe_item_unref((SpiceVmcPipeItem *)msg);
> -}
> -
>  static RedCharDeviceMsgToClient
> *spicevmc_chardev_read_msg_from_dev(SpiceCharDeviceInstance *sin,
>                                                                      void
> *opaque)
>  {
> @@ -614,8 +602,8 @@
> red_char_device_spicevmc_class_init(RedCharDeviceSpiceVmcClass *klass)
>      RedCharDeviceClass *char_dev_class = RED_CHAR_DEVICE_CLASS(klass);
>  
>      char_dev_class->read_one_msg_from_device =
> spicevmc_chardev_read_msg_from_dev;
> -    char_dev_class->ref_msg_to_client = spicevmc_chardev_ref_msg_to_client;
> -    char_dev_class->unref_msg_to_client =
> spicevmc_chardev_unref_msg_to_client;
> +    char_dev_class->ref_msg_to_client =
> (RedCharDeviceRefMsgToClient)spicevmc_pipe_item_ref;
> +    char_dev_class->unref_msg_to_client =
> (RedCharDeviceUnrefMsgToClient)spicevmc_pipe_item_unref;
>      char_dev_class->send_msg_to_client = spicevmc_chardev_send_msg_to_client;
>      char_dev_class->send_tokens_to_client =
> spicevmc_char_dev_send_tokens_to_client;
>      char_dev_class->remove_client = spicevmc_char_dev_remove_client;
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]