[Telepathy] review: Gabble hash caps

Dafydd Harries dafydd.harries at collabora.co.uk
Tue May 27 12:18:05 PDT 2008


First pass:

+  str = g_string_free (s, FALSE);
+  DEBUG ("caps string: '%s'", str);
+  sha1_bin (str, strlen (str), (guchar *) sha1);
+  encoded = base64_encode (SHA1_HASH_SIZE, sha1, FALSE);
+  DEBUG ("caps base64: '%s'", encoded);

Don't you leak str here? I think we can call sha1_bin (s->str, s->len, ...),
and then g_string_free (s, TRUE).

+          if (! g_str_equal (xmlns, "jabber:x:data"))
+            continue;
+
+          if (! g_str_equal (type, "result"))
+            continue;

These can crash if xmlns or type is NULL. Use g_strdiff instead.

-  uris = _extract_cap_bundles (lm_node);
+  uris = _extract_cap_bundles (lm_node, &hash, &ver);
+  DEBUG ("_extract_cap_bundles get hash='%s' ver='%s'", hash, ver);

Perhaps _extract_cap_bundles should be renamed to something like
_parse_caps_node?

+  /* Only 'sha-1' is mandatory to implement by XEP-0115. If the received
+   * discovery response uses another hash algorithm, don't check the hash and
+   * fallback to the old method. */
+  if (!tp_strdiff (waiter_self->hash, "sha-1"))

The comment doesn't seem to match the code here. It seems to be comparing
against the hash algorithm indicated in the original caps node, not one
indicated in the disco reply. Can you clarify?

+  waiter_self = NULL;
+  for (i = waiters; NULL != i;  i = i->next)

Perhaps we should be using a hash table here.

-        self.remote_caps = { 'ext': 'voice-v1 jingle-audio jingle-video', 'ver': '0.6.0',
-                 'node': 'http://telepathy.freedesktop.org/caps' }
+        self.remote_caps = { 'ext': '', 'ver': '0.0.0',
+                 'node': 'http://example.com/fake-client0' }

Hmm, was this causing some problem?

+    _test_without_hash(q, bus, conn, stream, 'bob1 at foo.com/Foo', 2L, client,
1)

s/1/True/.

+caps_changed_flag = 0

s/0/False/.


+def dbus_sync(bus, q, conn):
+    # Dummy D-Bus method call
+    call_async(q, conn, "InspectHandles", 1, [])
+
+    event = q.expect('dbus-return', method='InspectHandles')

I think I've written this before, but using org.freedesktop.Ping. Perhaps worth moving into servicetest.py.

-- 
Dafydd


More information about the Telepathy mailing list