[Bug 33404] Move caps hash stuff to Wocky

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Fri Feb 25 15:16:13 CET 2011


https://bugs.freedesktop.org/show_bug.cgi?id=33404

Will Thompson <will.thompson at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Status Whiteboard|                            |review-

--- Comment #8 from Will Thompson <will.thompson at collabora.co.uk> 2011-02-25 06:16:11 PST ---
“node: add attribute build tag for wocky_stanza_build”: I think you meant to
say “xml:lang attribute build tag” but I'll let it slide.


+      GSList *l;
+
+      for (l = node->children; l != NULL; l = l->next)
+        {
+          WockyNode *child = l->data;
+
+          if (!wocky_strdiff (child->name, "value"))
+            {
+              if (type == WOCKY_DATA_FORM_FIELD_TYPE_TEXT_SINGLE)
+                {
+                  type = WOCKY_DATA_FORM_FIELD_TYPE_TEXT_MULTI;
+                  break;
+                }
+              else
+                {
+                  type = WOCKY_DATA_FORM_FIELD_TYPE_TEXT_SINGLE;
+                }
+            }
+        }

type = WOCKY_DATA_FORM_FIELD_TYPE_TEXT_SINGLE;

WockyNodeIter iter;
wocky_node_iter_init (&iter, node, "value");

if (wocky_node_iter_next (&iter, NULL) &&
    wocky_node_iter_next (&iter, NULL))
  type = WOCKY_DATA_FORM_FIELD_TYPE_TEXT_MULTI;

? The patch as it stands would leave type = 0 in some cases?

+      field = g_hash_table_lookup (dataform->fields, "FORM_TYPE");
+      g_assert (field != NULL);

I don't see any reason why this assertion is valid, given that this is data
received over the network.

-      g_ptr_array_sort (form->fields, fields_cmp);
-          g_ptr_array_sort (field->values, char_cmp);

I don't see anything sorting the fields or their values in the new code. The
XEP says:

    1. Sort the fields by the value of the "var" attribute.

Ah you fixed this in 
And:

    1. Append the value of the "var" attribute, followed by the '<'
       character.
    2. Sort values by the XML character data of the <value/> element.
    3. For each <value/> element, append the XML character data,
       followed by the '<' character.

Please add a test case which hashes two forms which differ only in the order of
their values.

Also how does this deal with forms with fields of types other than text-single
or text-multi?

-  if (!source)
-    return NULL;
+  g_return_val_if_fail (source != NULL, NULL);


-  if (!arr)
+  if (arr == NULL)

Why does copy critical, but free merely early-return?

+  dataform = wocky_data_form_new_from_form (node, &error);
+  if (error != NULL)
+    {
+      DEBUG ("Failed to parse data form: %s\n", error->message);
+      g_clear_error (&error);
     }

We should fail the entire hashing process if the form is malformed, or we'll
get confused. *Particularly* if it's our own form that's malformed.


+      return strcmp (g_value_get_string (left_type->default_value),
+          g_value_get_string (left_type->default_value));

One of those should be 'right_type'. Also this will crash if someone specifies
>1 value for FORM_TYPE. I suppose parsing the form should fail in that case.
It'd be nice to have wocky_data_form_get_form_type ().

Fixed in a later commit, I see.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.


More information about the telepathy-bugs mailing list