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

> +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.

> 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 :)