[telepathy-gabble/master] Represent bare jids as resource = NULL, not resource = ""

Simon McVittie simon.mcvittie at collabora.co.uk
Wed Jan 6 05:16:55 PST 2010


The RFCs don't explicitly say anywhere that the empty string is not a
valid resource... also, it's cleaner this way. However, this does mean
changing a couple of function signatures.

Also move the definition of SESSION_MAP_KEY_FORMAT close to its only use.
---
 src/call-channel.c   |    8 ++----
 src/jingle-factory.c |   29 +++++++++++++++++-------
 src/jingle-session.c |   18 ++++++++++-----
 src/media-channel.c  |    9 +++----
 src/presence.c       |    1 -
 src/util.c           |   59 ++++++++++++++++++++++++++++----------------------
 src/util.h           |    5 ++-
 7 files changed, 75 insertions(+), 54 deletions(-)

diff --git a/src/call-channel.c b/src/call-channel.c
index 2fabd24..4ab260c 100644
--- a/src/call-channel.c
+++ b/src/call-channel.c
@@ -716,7 +716,7 @@ call_channel_create_content (GabbleCallChannel *self,
 
   peer_resource = gabble_jingle_session_get_peer_resource (priv->session);
 
-  if (peer_resource[0] != '\0')
+  if (peer_resource != NULL)
     DEBUG ("existing call, using peer resource %s", peer_resource);
   else
     DEBUG ("existing call, using bare JID");
@@ -1019,11 +1019,9 @@ call_channel_init_async (GAsyncInitable *initable,
 
       /* FIXME might need to wait on capabilities, also don't need transport
        * and dialect already */
-      resource = jingle_pick_best_resource (priv->conn,
+      if (!jingle_pick_best_resource (priv->conn,
         priv->target, priv->initial_audio, priv->initial_video,
-        &transport, &dialect);
-
-      if (resource == NULL)
+        &transport, &dialect, &resource))
         {
           g_simple_async_result_set_error (result, TP_ERRORS,
             TP_ERROR_NOT_CAPABLE,
diff --git a/src/jingle-factory.c b/src/jingle-factory.c
index 6ed6f4a..a32c472 100644
--- a/src/jingle-factory.c
+++ b/src/jingle-factory.c
@@ -58,11 +58,6 @@ enum
   LAST_PROPERTY
 };
 
-/* The 'session' map is keyed by:
- * "<peer's handle>\n<peer's resource>\n<session id>"
- */
-#define SESSION_MAP_KEY_FORMAT "%u\n%s\n%s"
-
 struct _GabbleJingleFactoryPrivate
 {
   GabbleConnection *conn;
@@ -684,12 +679,21 @@ connection_status_changed_cb (GabbleConnection *conn,
     }
 }
 
+/* The 'session' map is keyed by:
+ * "<peer's handle>\n<peer's resource>\n<session id>"
+ * or for bare JIDs
+ * "<peer's handle>\n\001\n<session id>"
+ * (\001 is not a valid character in a resource, as per resourceprep).
+ */
+#define SESSION_MAP_KEY_FORMAT "%u\n%s\n%s"
+
 static gchar *
 make_session_map_key (TpHandle peer,
     const gchar *resource,
     const gchar *sid)
 {
-  return g_strdup_printf (SESSION_MAP_KEY_FORMAT, peer, resource, sid);
+  return g_strdup_printf (SESSION_MAP_KEY_FORMAT, peer,
+      resource == NULL ? "\001" : resource, sid);
 }
 
 static gchar *
@@ -738,7 +742,7 @@ ensure_session (GabbleJingleFactory *self,
   if (resource == NULL || *resource == '\0')
     {
       /* if we're called by a SIP gateway, it might be using a bare JID */
-      resource = "";
+      resource = NULL;
     }
   else
     {
@@ -853,9 +857,9 @@ create_session (GabbleJingleFactory *fac,
   GabbleJingleSession *sess;
   gboolean local_initiator;
   gchar *sid_, *key;
+  const gchar *resource_sep;
 
   g_assert (peer != 0);
-  g_assert (peer_resource != NULL);
 
   if (sid != NULL)
     {
@@ -882,7 +886,14 @@ create_session (GabbleJingleFactory *fac,
   /* Takes ownership of key */
   g_hash_table_insert (priv->sessions, key, sess);
 
-  DEBUG ("new session (%u, '%s', %s) @ %p", peer, peer_resource, sid_, sess);
+  if (peer_resource == NULL)
+    resource_sep = "'";
+  else
+    resource_sep = "";
+
+  DEBUG ("new session (%u, %s%s%s, %s) @ %p", peer, resource_sep,
+      peer_resource == NULL ? "(no resource)" : peer_resource, resource_sep,
+      sid_, sess);
 
   g_free (sid_);
 
diff --git a/src/jingle-session.c b/src/jingle-session.c
index 5dde4cf..0556d18 100644
--- a/src/jingle-session.c
+++ b/src/jingle-session.c
@@ -349,7 +349,6 @@ gabble_jingle_session_constructed (GObject *object)
 
   g_assert (priv->conn != NULL);
   g_assert (self->peer != 0);
-  g_assert (priv->peer_resource != NULL);
   g_assert (priv->sid != NULL);
 
   tp_handle_ref (contact_repo, self->peer);
@@ -360,9 +359,16 @@ gabble_jingle_session_constructed (GObject *object)
    * should be two variants: one taking handle + resource, and another taking a
    * full JID; the other fields could be filled in here.
    */
-  priv->peer_jid = g_strdup_printf ("%s%s%s",
-      tp_handle_inspect (contact_repo, self->peer),
-      priv->peer_resource[0] == '\0' ? "" : "/", priv->peer_resource);
+  if (priv->peer_resource == NULL)
+    {
+      priv->peer_jid = g_strdup (tp_handle_inspect (contact_repo, self->peer));
+    }
+  else
+    {
+      priv->peer_jid = g_strdup_printf ("%s/%s",
+          tp_handle_inspect (contact_repo, self->peer),
+          priv->peer_resource);
+    }
 
   if (priv->local_initiator)
     priv->initiator = gabble_connection_get_full_jid (priv->conn);
@@ -428,7 +434,7 @@ gabble_jingle_session_class_init (GabbleJingleSessionClass *cls)
 
   param_spec = g_param_spec_string ("peer-resource", "Session peer's resource",
       "The resource of the contact with whom this session communicates, "
-      "or an empty string if communicating with a bare JID",
+      "or NULL if communicating with a bare JID",
       NULL,
       G_PARAM_CONSTRUCT_ONLY | G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS);
   g_object_class_install_property (object_class, PROP_PEER_RESOURCE,
@@ -633,7 +639,7 @@ lookup_content (GabbleJingleSession *sess,
           priv->conn->presence_cache, sess->peer);
 
       if (creator == NULL && presence != NULL &&
-          priv->peer_resource[0] != '\0' &&
+          priv->peer_resource != NULL &&
           gabble_presence_resource_has_caps (presence, priv->peer_resource,
               gabble_capability_set_predicate_has,
               QUIRK_OMITS_CONTENT_CREATORS))
diff --git a/src/media-channel.c b/src/media-channel.c
index 42ab758..75b0b17 100644
--- a/src/media-channel.c
+++ b/src/media-channel.c
@@ -1581,10 +1581,8 @@ _gabble_media_channel_request_contents (GabbleMediaChannel *chan,
 
       g_assert (priv->streams->len == 0);
 
-      peer_resource = jingle_pick_best_resource (priv->conn, peer,
-          want_audio, want_video, &transport_ns, &dialect);
-
-      if (peer_resource == NULL)
+      if (!jingle_pick_best_resource (priv->conn, peer,
+          want_audio, want_video, &transport_ns, &dialect, &peer_resource))
         {
           g_set_error (error, TP_ERRORS, TP_ERROR_NOT_CAPABLE,
               "member does not have the desired audio/video capabilities");
@@ -1592,7 +1590,8 @@ _gabble_media_channel_request_contents (GabbleMediaChannel *chan,
         }
 
       DEBUG ("Picking resource '%s' (transport: %s, dialect: %u)",
-          peer_resource, transport_ns, dialect);
+          peer_resource == NULL ? "(null)" : peer_resource,
+          transport_ns, dialect);
 
       create_session (chan, peer, peer_resource);
 
diff --git a/src/presence.c b/src/presence.c
index 47549b1..06e1833 100644
--- a/src/presence.c
+++ b/src/presence.c
@@ -648,7 +648,6 @@ gabble_presence_resource_pick_best_feature (GabblePresence *presence,
 
   g_return_val_if_fail (presence != NULL, NULL);
   g_return_val_if_fail (resource != NULL, NULL);
-  g_return_val_if_fail (resource[0] != '\0', NULL);
   g_return_val_if_fail (predicate != NULL, NULL);
   g_return_val_if_fail (table != NULL, NULL);
 
diff --git a/src/util.c b/src/util.c
index 996e352..ab501f6 100644
--- a/src/util.c
+++ b/src/util.c
@@ -1031,33 +1031,44 @@ ensure_bare_contact_from_jid (GabbleConnection *conn,
 
 #define TWICE(x) x, x
 
-static const gchar *
+static gboolean
 jingle_pick_resource_or_bare_jid (GabblePresence *presence,
-    GabbleCapabilitySet *caps)
+    GabbleCapabilitySet *caps, const gchar **resource)
 {
+  const gchar *ret;
+
   if (gabble_presence_has_resources (presence))
     {
-      return gabble_presence_pick_resource_by_caps (presence,
+      ret = gabble_presence_pick_resource_by_caps (presence,
           gabble_capability_set_predicate_at_least, caps);
+
+      if (resource != NULL)
+        *resource = ret;
+
+      return (ret != NULL);
     }
   else if (gabble_capability_set_at_least (
         gabble_presence_peek_caps (presence), caps))
     {
-      return "";
+      if (resource != NULL)
+        *resource = NULL;
+
+      return TRUE;
     }
   else
     {
-      return NULL;
+      return FALSE;
     }
 }
 
-const gchar *
+gboolean
 jingle_pick_best_resource (GabbleConnection *conn,
     TpHandle peer,
     gboolean want_audio,
     gboolean want_video,
     const char **transport_ns,
-    JingleDialect *dialect)
+    JingleDialect *dialect,
+    const gchar **resource_out)
 {
   /* We prefer gtalk-p2p to ice, because it can use tcp and https relays (if
    * available). */
@@ -1070,19 +1081,20 @@ jingle_pick_best_resource (GabbleConnection *conn,
   GabblePresence *presence;
   GabbleCapabilitySet *caps;
   const gchar *resource = NULL;
+  gboolean success = FALSE;
 
   presence = gabble_presence_cache_get (conn->presence_cache, peer);
 
   if (presence == NULL)
     {
       DEBUG ("contact %d has no presence available", peer);
-      return NULL;
+      return FALSE;
     }
 
   *dialect = JINGLE_DIALECT_ERROR;
   *transport_ns = NULL;
 
-  g_return_val_if_fail (want_audio || want_video, NULL);
+  g_return_val_if_fail (want_audio || want_video, FALSE);
 
   /* from here on, goto FINALLY to free this, instead of returning early */
   caps = gabble_capability_set_new ();
@@ -1095,9 +1107,7 @@ jingle_pick_best_resource (GabbleConnection *conn,
   if (want_video)
     gabble_capability_set_add (caps, NS_JINGLE_RTP_VIDEO);
 
-  resource = jingle_pick_resource_or_bare_jid (presence, caps);
-
-  if (resource != NULL)
+  if (jingle_pick_resource_or_bare_jid (presence, caps, &resource))
     {
       *dialect = JINGLE_DIALECT_V032;
       goto CHOOSE_TRANSPORT;
@@ -1111,9 +1121,7 @@ jingle_pick_best_resource (GabbleConnection *conn,
   if (want_video)
     gabble_capability_set_add (caps, NS_JINGLE_DESCRIPTION_VIDEO);
 
-  resource = jingle_pick_resource_or_bare_jid (presence, caps);
-
-  if (resource != NULL)
+  if (jingle_pick_resource_or_bare_jid (presence, caps, &resource))
     {
       *dialect = JINGLE_DIALECT_V015;
       goto CHOOSE_TRANSPORT;
@@ -1133,9 +1141,7 @@ jingle_pick_best_resource (GabbleConnection *conn,
   if (want_video)
     gabble_capability_set_add (caps, NS_GOOGLE_FEAT_VIDEO);
 
-  resource = jingle_pick_resource_or_bare_jid (presence, caps);
-
-  if (resource != NULL)
+  if (jingle_pick_resource_or_bare_jid (presence, caps, &resource))
     {
       *dialect = JINGLE_DIALECT_GTALK3;
       goto CHOOSE_TRANSPORT;
@@ -1151,9 +1157,8 @@ jingle_pick_best_resource (GabbleConnection *conn,
   gabble_capability_set_clear (caps);
   gabble_capability_set_add (caps, NS_GOOGLE_FEAT_VOICE);
   gabble_capability_set_add (caps, NS_GOOGLE_TRANSPORT_P2P);
-  resource = jingle_pick_resource_or_bare_jid (presence, caps);
 
-  if (resource != NULL)
+  if (jingle_pick_resource_or_bare_jid (presence, caps, &resource))
     {
       *dialect = JINGLE_DIALECT_GTALK4;
       goto CHOOSE_TRANSPORT;
@@ -1164,13 +1169,17 @@ jingle_pick_best_resource (GabbleConnection *conn,
 
 CHOOSE_TRANSPORT:
 
+  if (resource_out != NULL)
+    *resource_out = resource;
+
+  success = TRUE;
 
   if (*dialect == JINGLE_DIALECT_GTALK4 || *dialect == JINGLE_DIALECT_GTALK3)
     {
       /* the GTalk dialects only support google p2p as transport protocol. */
       *transport_ns = NS_GOOGLE_TRANSPORT_P2P;
     }
-  else if (resource[0] == '\0')
+  else if (resource == NULL)
     {
       *transport_ns = gabble_presence_pick_best_feature (presence, transports,
           gabble_capability_set_predicate_has);
@@ -1182,11 +1191,11 @@ CHOOSE_TRANSPORT:
     }
 
   if (*transport_ns == NULL)
-    resource = NULL;
+    success = FALSE;
 
 FINALLY:
   gabble_capability_set_free (caps);
-  return resource;
+  return success;
 }
 
 const gchar *
@@ -1211,8 +1220,6 @@ jingle_pick_best_content_type (GabbleConnection *conn,
         { FALSE, NULL, NULL }
   };
 
-  g_return_val_if_fail (resource != NULL, NULL);
-
   presence = gabble_presence_cache_get (conn->presence_cache, peer);
 
   if (presence == NULL)
@@ -1221,7 +1228,7 @@ jingle_pick_best_content_type (GabbleConnection *conn,
       return NULL;
     }
 
-  if (resource[0] == '\0')
+  if (resource == NULL)
     {
       return gabble_presence_pick_best_feature (presence, content_types,
           gabble_capability_set_predicate_has);
diff --git a/src/util.h b/src/util.h
index e724f74..aa6dbe6 100644
--- a/src/util.h
+++ b/src/util.h
@@ -104,12 +104,13 @@ GPtrArray *gabble_g_ptr_array_copy (GPtrArray *source);
 WockyBareContact * ensure_bare_contact_from_jid (GabbleConnection *conn,
     const gchar *jid);
 
-const gchar * jingle_pick_best_resource (GabbleConnection *conn,
+gboolean jingle_pick_best_resource (GabbleConnection *conn,
     TpHandle peer,
     gboolean want_audio,
     gboolean want_video,
     const char **transport_ns,
-    JingleDialect *dialect);
+    JingleDialect *dialect,
+    const gchar **resource_out);
 
 const gchar *jingle_pick_best_content_type (GabbleConnection *conn,
     TpHandle peer,
-- 
1.5.6.5



More information about the telepathy-commits mailing list