[libvirt] [libvirt-glib 04/37] Add some GVirConfigClock setters

[Marc-André Lureau]

Hi

On Thu, Nov 10, 2011 at 9:33 PM, Christophe Fergeau <cfergeau at redhat.com> wrote:
> ---
>  libvirt-gconfig/libvirt-gconfig-clock.c  |   32 ++++++++++++++++++++++++++++++
>  libvirt-gconfig/libvirt-gconfig-clock.h  |    5 ++++
>  libvirt-gconfig/libvirt-gconfig-domain.c |    1 +
>  libvirt-gconfig/libvirt-gconfig-domain.h |    1 -
>  libvirt-gconfig/libvirt-gconfig.sym      |    2 +
>  5 files changed, 40 insertions(+), 1 deletions(-)
>
> diff --git a/libvirt-gconfig/libvirt-gconfig-clock.c b/libvirt-gconfig/libvirt-gconfig-clock.c
> index d2650ad..120d3a6 100644
> --- a/libvirt-gconfig/libvirt-gconfig-clock.c
> +++ b/libvirt-gconfig/libvirt-gconfig-clock.c
> @@ -80,3 +80,35 @@ GVirConfigClock *gvir_config_clock_new_from_xml(const gchar *xml,
>                                              "clock", NULL, xml, error);
>     return GVIR_CONFIG_CLOCK(object);
>  }
> +
> +void gvir_config_clock_set_timezone(GVirConfigClock *klock,
> +                                    const char *tz)
> +{
> +    xmlNodePtr node;
> +    xmlChar *encoded_tz;

in general, I think we should add g_return_..if_fail() for all public
functions, and all arguments. Would you mind adding it in your
patches?

> +
> +    node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(klock));
> +    if (node == NULL)
> +        return;
> +
> +    xmlNewProp(node, (xmlChar*)"offset", (xmlChar*)"timezone");
> +    encoded_tz = xmlEncodeEntitiesReentrant(node->doc, (xmlChar*)tz);
> +    xmlNewProp(node, (xmlChar*)"timezone", encoded_tz);
> +    xmlFree(encoded_tz);

the (xmlChar*) and (gchar*) casts are very frequent in the code, and
make the code harder to read. I wonder if we should have macro such as
XCHAR() GCHAR(). I think I have since that in other projects.

> +
> +void gvir_config_clock_set_variable_offset(GVirConfigClock *klock,
> +                                           gint seconds)
> +{
> +    xmlNodePtr node;
> +    char *offset_str;
> +
> +    node = gvir_config_object_new_child(GVIR_CONFIG_OBJECT(klock), "clock");
> +    if (node == NULL)
> +        return;
> +
> +    xmlNewProp(node, (xmlChar*)"offset", (xmlChar*)"variable");
> +    offset_str = g_strdup_printf("%d", seconds);
> +    xmlNewProp(node, (xmlChar*)"timezone", (xmlChar*)offset_str);
> +    g_free(offset_str);
> +}
> diff --git a/libvirt-gconfig/libvirt-gconfig-clock.h b/libvirt-gconfig/libvirt-gconfig-clock.h
> index fc8850a..26f4b53 100644
> --- a/libvirt-gconfig/libvirt-gconfig-clock.h
> +++ b/libvirt-gconfig/libvirt-gconfig-clock.h
> @@ -63,6 +63,11 @@ GVirConfigClock *gvir_config_clock_new(void);
>  GVirConfigClock *gvir_config_clock_new_from_xml(const gchar *xml,
>                                                 GError **error);
>
> +void gvir_config_clock_set_timezone(GVirConfigClock *klock,
> +                                    const char *tz);
> +void gvir_config_clock_set_variable_offset(GVirConfigClock *klock,
> +                                           gint seconds);
> +
>  G_END_DECLS
>
>  #endif /* __LIBVIRT_GCONFIG_CLOCK_H__ */
> diff --git a/libvirt-gconfig/libvirt-gconfig-domain.c b/libvirt-gconfig/libvirt-gconfig-domain.c
> index c847c14..f80720a 100644
> --- a/libvirt-gconfig/libvirt-gconfig-domain.c
> +++ b/libvirt-gconfig/libvirt-gconfig-domain.c
> @@ -236,3 +236,4 @@ void gvir_config_domain_set_features(GVirConfigDomain *domain,
>     }
>     g_object_notify(G_OBJECT(domain), "features");
>  }
> +

Extra line?

> diff --git a/libvirt-gconfig/libvirt-gconfig-domain.h b/libvirt-gconfig/libvirt-gconfig-domain.h
> index d9f0c09..6cc8f31 100644
> --- a/libvirt-gconfig/libvirt-gconfig-domain.h
> +++ b/libvirt-gconfig/libvirt-gconfig-domain.h
> @@ -70,7 +70,6 @@ GStrv gvir_config_domain_get_features(GVirConfigDomain *domain);
>  void gvir_config_domain_set_features(GVirConfigDomain *domain,
>                                      const GStrv features);
>
> -

ok :)

-- 
Marc-André Lureau