[Spice-devel] [PATCH spice-server v3 4/5] red-channel-capabilities: Enhance

[Christophe Fergeau]

On Thu, Mar 02, 2017 at 09:16:10AM +0000, Frediano Ziglio wrote:
> Add function to initialize and destroy this type.
> Add GObject type for boxing it.

Nit: I would not call 'GObject type' a GType/GBoxed GObject.


> These changes a in preparation for next patch.
> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  server/red-channel-capabilities.c | 55 +++++++++++++++++++++++++++++++++++++++
>  server/red-channel-capabilities.h | 16 +++++++++++-
>  2 files changed, 70 insertions(+), 1 deletion(-)
> 
> diff --git a/server/red-channel-capabilities.c b/server/red-channel-capabilities.c
> index 39bde66..dc28298 100644
> --- a/server/red-channel-capabilities.c
> +++ b/server/red-channel-capabilities.c
> @@ -18,5 +18,60 @@
>  #include <config.h>
>  #endif
>  
> +#include <string.h>
> +#include <common/mem.h>
> +#include <common/macros.h>
> +
>  #include "red-channel-capabilities.h"
>  
> +GType red_channel_capabilities_type;
> +
> +void red_channel_capabilities_init(RedChannelCapabilities *dest,
> +                                   const RedChannelCapabilities *caps)
> +{
> +    *dest = *caps;
> +    if (caps->common_caps) {
> +        dest->common_caps = spice_memdup(caps->common_caps,
> +                                         caps->num_common_caps * sizeof(uint32_t));
> +    }
> +    if (caps->num_caps) {
> +        dest->caps = spice_memdup(caps->caps, caps->num_caps * sizeof(uint32_t));
> +    }
> +}
> +
> +void red_channel_capabilities_reset(RedChannelCapabilities *caps)
> +{
> +    if (caps->num_common_caps) {
> +        free(caps->common_caps);
> +    }
> +
> +    if (caps->num_caps) {
> +        free(caps->caps);
> +    }
> +    memset(caps, 0, sizeof(*caps));
> +}
> +
> +static RedChannelCapabilities *red_channel_capabilities_dup(const RedChannelCapabilities *caps)
> +{
> +    if (!caps) {
> +        return NULL;
> +    }

It's apparently invalid to return NULL from a GBoxedCopyFunc. Dunno if
the 'caps' argument can validly be NULL or not, probably not?

> +
> +    RedChannelCapabilities *res = spice_new(RedChannelCapabilities, 1);
> +    red_channel_capabilities_init(res, caps);
> +    return res;
> +}
> +
> +static void red_channel_capabilities_free(RedChannelCapabilities *caps)
> +{
> +    red_channel_capabilities_reset(caps);
> +    free(caps);
> +}
> +
> +SPICE_CONSTRUCTOR_FUNC(red_channel_capabilities_construct)
> +{
> +    red_channel_capabilities_type =
> +        g_boxed_type_register_static("red_channel_capabilities_type",
> +                                     (GBoxedCopyFunc) red_channel_capabilities_dup,
> +                                     (GBoxedFreeFunc) red_channel_capabilities_free);
> +}
> diff --git a/server/red-channel-capabilities.h b/server/red-channel-capabilities.h
> index 8729134..1f52cd4 100644
> --- a/server/red-channel-capabilities.h
> +++ b/server/red-channel-capabilities.h
> @@ -20,10 +20,13 @@
>  #define RED_CHANNEL_CAPABILITIES_H_
>  
>  #include <stdint.h>
> -#include <glib.h>
> +#include <glib-object.h>
>  
>  G_BEGIN_DECLS
>  
> +/* Holds channel capabilities.
> + * Channel capabilities are composed by a common part
> + * and a specific one. */
>  typedef struct RedChannelCapabilities {
>      int num_common_caps;
>      uint32_t *common_caps;
> @@ -31,6 +34,17 @@ typedef struct RedChannelCapabilities {
>      uint32_t *caps;
>  } RedChannelCapabilities;
>  
> +/* Initialize the structure based on a previous one. */
> +void red_channel_capabilities_init(RedChannelCapabilities *dest,
> +                                   const RedChannelCapabilities *caps);
> +
> +/* Reset capabilities.
> + * All resources are freed by this function. */
> +void red_channel_capabilities_reset(RedChannelCapabilities *caps);
> +
> +/* GObject type that can be used to box RedChannelCapabilities */
> +extern GType red_channel_capabilities_type;

Usually a _get_type() method is exported, together with a #define.
You favoured using a static constructor, but I'd still add a
#define RED_TYPE_CHANNEL_CAPABILITIES red_channel_capabilities_type
for consistency with the other GType names.


Looks good to me.

Christophe

> +
>  G_END_DECLS
>  
>  #endif
> -- 
> 2.9.3
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170302/0e460ae2/attachment-0001.sig>