[PATCH] Simplify XML document creation utility functions

[Adrian Perez]

Both _xml_add_string_child() and _xml_add_boolean_child() accept now a
value, instead of reading it from a GObject's property. This way of making
things has some advantages:

* Simpler, so it is a bit easier to understand.

* Generic implementation: values to save inside tags can be directly passed
instead of needing a property of a GObject.

* Avoids using g_object_get() so we do less string allocations.
---
src/frogr-config.c | 58 +++++++++++++++++++++------------------------------
1 files changed, 24 insertions(+), 34 deletions(-)

diff --git a/src/frogr-config.c b/src/frogr-config.c
index 6160317..f7cac31 100644
--- a/src/frogr-config.c
+++ b/src/frogr-config.c
@@ -198,26 +198,20 @@ _frogr_config_load (FrogrConfig *fconfig, const gchar *config_dir)

static xmlNodePtr
_xml_add_string_child (xmlNodePtr parent,
- const gchar *xml_name,
- GObject *object,
- const gchar *prop_name)
+ const gchar *tag_name,
+ const gchar *value)
{
xmlNodePtr node;
- xmlChar *enc;
- gchar *value;
+ xmlChar *encoded;

- g_return_val_if_fail (parent != NULL, NULL);
- g_return_val_if_fail (xml_name != NULL, NULL);
- g_return_val_if_fail (object != NULL, NULL);
- g_return_val_if_fail (prop_name != NULL, NULL);
-
- g_object_get (object, prop_name, &value, NULL);
+ g_return_val_if_fail (parent != NULL, NULL);
+ g_return_val_if_fail (tag_name != NULL, NULL);
+ g_return_val_if_fail (value != NULL, NULL);

- node = xmlNewNode (NULL, (const xmlChar*) xml_name);
- enc = xmlEncodeEntitiesReentrant (NULL, (const xmlChar*) value);
- xmlNodeSetContent (node, enc);
- g_free (value);
- xmlFree (enc);
+ node = xmlNewNode (NULL, (const xmlChar*) tag_name);
+ encoded = xmlEncodeEntitiesReentrant (NULL, (const xmlChar*) value);
+ xmlNodeSetContent (node, encoded);
+ xmlFree (encoded);
xmlAddChild (parent, node);

return node;
@@ -225,21 +219,15 @@ _xml_add_string_child (xmlNodePtr parent,

static xmlNodePtr
_xml_add_boolean_child (xmlNodePtr parent,
- const gchar *xml_name,
- GObject *object,
- const gchar *prop_name)
+ const gchar *tag_name,
+ gboolean value)
{
xmlNodePtr node;
- gboolean value;

g_return_val_if_fail (parent != NULL, NULL);
- g_return_val_if_fail (xml_name != NULL, NULL);
- g_return_val_if_fail (object != NULL, NULL);
- g_return_val_if_fail (prop_name != NULL, NULL);
-
- g_object_get (object, prop_name, &value, NULL);
+ g_return_val_if_fail (tag_name != NULL, NULL);

- node = xmlNewNode (NULL, (const xmlChar*) xml_name);
+ node = xmlNewNode (NULL, (const xmlChar*) tag_name);
xmlNodeSetContent (node, (const xmlChar*) ((value) ? "true" : "false"));
xmlAddChild (parent, node);

@@ -254,7 +242,6 @@ _frogr_config_save_accounts (FrogrConfig *fconfig)
xmlDocPtr xml;
xmlNodePtr node, root;
GList *item;
- GObject *account;
gchar *xml_path;

g_return_val_if_fail (FROGR_IS_CONFIG (fconfig), FALSE);
@@ -269,15 +256,18 @@ _frogr_config_save_accounts (FrogrConfig *fconfig)
item != NULL;
item = g_list_next (item))
{
- account = G_OBJECT (item->data);
+ FrogrAccount *faccount = FROGR_ACCOUNT (item -> data);
+
+ g_print ("tok: %s\n", frogr_account_get_token (faccount));
+
node = xmlNewNode (NULL, (const xmlChar*) "account");
- _xml_add_string_child (node, "frob", account, "frob");
- _xml_add_string_child (node, "token", account, "token");
- _xml_add_string_child (node, "username", account, "username");
+ _xml_add_string_child (node, "frob", frogr_account_get_frob (faccount));
+ _xml_add_string_child (node, "token", frogr_account_get_token (faccount));
+ _xml_add_string_child (node, "username", frogr_account_get_username (faccount));

- _xml_add_boolean_child (node, "public", account, "public");
- _xml_add_boolean_child (node, "family", account, "family");
- _xml_add_boolean_child (node, "friends", account, "friends");
+ _xml_add_boolean_child (node, "public", frogr_account_get_public (faccount));
+ _xml_add_boolean_child (node, "family", frogr_account_get_private_family (faccount));
+ _xml_add_boolean_child (node, "friends", frogr_account_get_private_friends (faccount));

xmlAddChild (root, node);
}
--
1.6.3.2

[Adrian Perez de Castro]

On Tue, 16 Jun 2009 01:58:09 +0200, Adrian wrote:

> Both _xml_add_string_child() and _xml_add_boolean_child() accept now a
> value, instead of reading it from a GObject's property. This way of making
> things has some advantages:
>

> [...]

Please forget about this patch. It has a g_print() which I used for
debugging. I will re-send it in some minutes.

--
Adrian Perez de Castro <ape...@igalia.com>
Igalia - Free Software Engineering

[Adrian Perez]

Both _xml_add_string_child() and _xml_add_boolean_child() accept now a
value, instead of reading it from a GObject's property. This way of making
things has some advantages:

* Simpler, so it is a bit easier to understand.

[Adrian Perez de Castro]

On Tue, 16 Jun 2009 02:01:07 +0200, Adrian wrote:

> Both _xml_add_string_child() and _xml_add_boolean_child() accept now a
> value, instead of reading it from a GObject's property. This way of making
> things has some advantages:
>

> [...]

I have sent the same patch again with the g_print() in it :-(

It looks like it is time to go to sleep... Sorry for the noise, and
forgot about this one as well.

[Adrian Perez]

Both _xml_add_string_child() and _xml_add_boolean_child() accept now a
value, instead of reading it from a GObject's property. This way of making
things has some advantages:

* Simpler, so it is a bit easier to understand.

* Generic implementation: values to save inside tags can be directly passed
instead of needing a property of a GObject.

* Avoids using g_object_get() so we do less string allocations.
---

src/frogr-config.c | 56 ++++++++++++++++++++-------------------------------
1 files changed, 22 insertions(+), 34 deletions(-)

diff --git a/src/frogr-config.c b/src/frogr-config.c
index 6160317..b6a98c2 100644

@@ -269,15 +256,16 @@ _frogr_config_save_accounts (FrogrConfig *fconfig)

item != NULL;
item = g_list_next (item))
{
- account = G_OBJECT (item->data);
+ FrogrAccount *faccount = FROGR_ACCOUNT (item -> data);
+

[Mario Sanchez Prada]

Adrian Perez wrote:
> static xmlNodePtr
> _xml_add_string_child (xmlNodePtr parent,
> - const gchar *xml_name,
> - GObject *object,
> - const gchar *prop_name)
> + const gchar *tag_name,
> + const gchar *value)

Shouldn't the last two params be (const xmlChar *) to avoid further
casting in the code (and to be more coherent, btw)?

> [...]

> - enc = xmlEncodeEntitiesReentrant (NULL, (const xmlChar*) value);

> [...]

> + node = xmlNewNode (NULL, (const xmlChar*) tag_name);
> + encoded = xmlEncodeEntitiesReentrant (NULL, (const xmlChar*) value);

... so you would remove those ugly casts from here.

> static xmlNodePtr
> _xml_add_boolean_child (xmlNodePtr parent,
> - const gchar *xml_name,
> - GObject *object,
> - const gchar *prop_name)
> + const gchar *tag_name,
> + gboolean value)

Same thing here.

> + g_print ("tok: %s\n", frogr_account_get_token (faccount));

You can change it for a g_debug and leave it in the patch, if you wish.
In general, you should use g_debug() for these kind of things and
g_print() basically just for more ad-hoc situations.

Thanks, I'll integrate it as soon as you send it corrected.

Mario

[Adrian Perez de Castro]

On Tue, 16 Jun 2009 07:59:02 +0100, Mario wrote:

>
> Adrian Perez wrote:
> > static xmlNodePtr
> > _xml_add_string_child (xmlNodePtr parent,
> > - const gchar *xml_name,
> > - GObject *object,
> > - const gchar *prop_name)
> > + const gchar *tag_name,
> > + const gchar *value)
>
> Shouldn't the last two params be (const xmlChar *) to avoid further
> casting in the code (and to be more coherent, btw)?
>
> > [...]
> > - enc = xmlEncodeEntitiesReentrant (NULL, (const xmlChar*) value);
> > [...]
> > + node = xmlNewNode (NULL, (const xmlChar*) tag_name);
> > + encoded = xmlEncodeEntitiesReentrant (NULL, (const xmlChar*) value);
>
> ... so you would remove those ugly casts from here.

I would like to, but the compiler will generate warnings about
different sign in gchar and xmlChar:

in glib/gtypes.h:
typedef char gchar;

in libxml/xmlstring.h:
typedef unsigned char xmlChar;

So depending on compiler version and platform, gchar can be signed or
unsigned, so the only way to avoid compiler warnings (and sometimes
range problems in code) is to *always* do a cast when going from gchar
to xmlChar, or xmlChar to gchar :P

:P