Re: [gupnp] [PATCH] Fix syntax of property sets with multiple properties.

["Zeeshan Ali (Khattak)" <zeenix gmail com>]

Hi,

On Mon, May 17, 2010 at 6:50 PM, Sven Neumann <s neumann raumfeld com> wrote:
> On Mon, 2010-05-17 at 18:45 +0300, Zeeshan Ali (Khattak) wrote:
>
>>   I agree but the other reason to remove that loop was to make the
>> very ugly code somewhat readable.  So Ross, please don't just revert
>> yet, I promise to provide a cleaner patch this week. If I don't
>> full-fill my promise please do a simple revert then. :)
>
> Thanks. If you don't get to it, just let me know. I should be able to
> find some time this week to clean up this code and make it backward
> compatible at the same time.

  No worry, patches attached. :)

-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124

From 0a00a56d2a7cc50d79f535e0588da0e862b799f9 Mon Sep 17 00:00:00 2001
From: Zeeshan Ali (Khattak) <zeeshanak gnome org>
Date: Mon, 17 May 2010 19:55:13 +0300
Subject: [PATCH 1/2] Refactor emit_notification function

---
 libgupnp/gupnp-service-proxy.c |   81 +++++++++++++++++++++------------------
 1 files changed, 44 insertions(+), 37 deletions(-)

diff --git a/libgupnp/gupnp-service-proxy.c b/libgupnp/gupnp-service-proxy.c
index 1e0b5f3..9db8634 100644
--- a/libgupnp/gupnp-service-proxy.c
+++ b/libgupnp/gupnp-service-proxy.c
@@ -1581,55 +1581,61 @@ gupnp_service_proxy_remove_notify (GUPnPServiceProxy              *proxy,
         return found;
 }
 
-static void emit_notification (GUPnPServiceProxy *proxy,
-                               xmlDoc            *doc)
+static void
+emit_notification (GUPnPServiceProxy *proxy,
+                   xmlNode           *var_node)
 {
-        xmlNode *node;
+        NotifyData *data;
+        GValue value = {0, };
+        GList *l;
 
-        node = xmlDocGetRootElement (doc);
+        data = g_hash_table_lookup (proxy->priv->notify_hash, var_node->name);
+        if (data == NULL)
+                return;
 
-        /* Iterate over all provided properties */
-        for (node = node->children; node; node = node->next) {
-                xmlNode *var_node;
-                NotifyData *data;
-                GValue value = {0, };
-                GList *l;
+        /* Make a GValue of the desired type */
+        g_value_init (&value, data->type);
 
-                /* variableName node */
-                var_node = node->children;
+        if (!gvalue_util_set_value_from_xml_node (&value, var_node)) {
+                g_value_unset (&value);
+
+                return;
+        }
 
-                if (var_node == NULL ||
-                    strcmp ((char *) node->name, "property") != 0)
-                        continue;
+        /* Call callbacks */
+        for (l = data->callbacks; l; l = l->next) {
+                CallbackData *callback_data;
 
-                data = g_hash_table_lookup (proxy->priv->notify_hash,
-                                            var_node->name);
-                if (data == NULL)
-                        continue;
+                callback_data = l->data;
 
-                /* Make a GValue of the desired type */
-                g_value_init (&value, data->type);
+                callback_data->callback (proxy,
+                                         (const char *) var_node->name,
+                                         &value,
+                                         callback_data->user_data);
+        }
 
-                if (!gvalue_util_set_value_from_xml_node (&value, var_node)) {
-                        g_value_unset (&value);
+        /* Cleanup */
+        g_value_unset (&value);
+}
 
-                        continue;
-                }
+static void
+emit_notifications_for_doc (GUPnPServiceProxy *proxy,
+                            xmlDoc            *doc)
+{
+        xmlNode *node;
 
-                /* Call callbacks */
-                for (l = data->callbacks; l; l = l->next) {
-                        CallbackData *callback_data;
+        node = xmlDocGetRootElement (doc);
 
-                        callback_data = l->data;
+        /* Iterate over all provided properties */
+        for (node = node->children; node; node = node->next) {
+                xmlNode *var_node;
 
-                        callback_data->callback (proxy,
-                                                 (const char *) var_node->name,
-                                                 &value,
-                                                 callback_data->user_data);
-                }
+                /* variableName node */
+                var_node = node->children;
 
-                /* Cleanup */
-                g_value_unset (&value);
+                if (var_node != NULL &&
+                    strcmp ((char *) node->name, "property") == 0)
+                        emit_notification (proxy, var_node);
         }
 }
 
@@ -1673,7 +1679,8 @@ emit_notifications (gpointer user_data)
                               strcmp (emit_notify_data->sid,
                                       proxy->priv->sid) == 0))
                         /* Our SID, entertain! */
-                        emit_notification (proxy, emit_notify_data->doc);
+                        emit_notifications_for_doc (proxy,
+                                                    emit_notify_data->doc);
         }
 
         /* Cleanup */
-- 
1.7.0.4

From 7400c9f9f3dded17af17533c6c8545d0c51f9993 Mon Sep 17 00:00:00 2001
From: Zeeshan Ali (Khattak) <zeeshanak gnome org>
Date: Mon, 17 May 2010 20:06:18 +0300
Subject: [PATCH 2/2] Handle multiple variables inside property node

Although we stopped doing this for very good reasons in commit 7ebd4453,
we are now re-introducing this to be compatible with server-side
implementations using older GUPnP.
---
 libgupnp/gupnp-service-proxy.c |   17 +++++++++++------
 1 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/libgupnp/gupnp-service-proxy.c b/libgupnp/gupnp-service-proxy.c
index 9db8634..33e06f0 100644
--- a/libgupnp/gupnp-service-proxy.c
+++ b/libgupnp/gupnp-service-proxy.c
@@ -1630,12 +1630,17 @@ emit_notifications_for_doc (GUPnPServiceProxy *proxy,
         for (node = node->children; node; node = node->next) {
                 xmlNode *var_node;
 
-                /* variableName node */
-                var_node = node->children;
-
-                if (var_node != NULL &&
-                    strcmp ((char *) node->name, "property") == 0)
-                        emit_notification (proxy, var_node);
+                /* Although according to the UPnP specs, there should be only
+                 * one variable node inside a 'property' node, we still need to
+                 * entertain the possibility of multiple variables inside it to
+                 * be compatible with implementations using older GUPnP.
+                 */
+                for (var_node = node->children;
+                     var_node;
+                     var_node = var_node->next) {
+                        if (strcmp ((char *) node->name, "property") == 0)
+                                emit_notification (proxy, var_node);
+                }
         }
 }
 
-- 
1.7.0.4