[Bug 34931] Add a "meta porter"

[bugzilla-daemon at freedesktop.org]

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

--- Comment #4 from Will Thompson <will.thompson at collabora.co.uk> 2011-03-15 08:08:44 PDT ---
- * Returns a #GList of #GInetSocketAddress<!-- -->es which relate to
- * @self. Note that the #GInetSocketAddresses should be unreffed by
- * calling g_object_unref() on each list member and the list freed
- * using g_list_free() when the caller is finished.
+ * Returns a #GList of #GInetSocketAddress<!-- -->es which are
+ * advertised by the contact @self as addresses to connect on. Note
+ * that the #GInetSocketAddresses should be unreffed by calling
+ * g_object_unref() on each list member and the list freed using
+ * g_list_free() when the caller is finished.
  *
  * Returns: (element-type GInetSocketAddress) (transfer full): a new
  *   #GList of #GInetSocketAddress<!-- -->es.

I really meant to add the annotation and remove the longhand description of it
from the docstring; but okay.


-  data->addresses = wocky_ll_contact_get_addresses (contact);
+  data->addresses = g_queue_new ();
+
+  addr = wocky_ll_contact_get_addresses (contact);
+  g_list_foreach (addr, add_to_queue, data->addresses);
+  g_list_free (addr);

Would it be too evil to manually initialize the queue's members? :p (I'm
disappointed that GQueue doesn't have an append_list() method which takes
ownership of an existing GList. Oh well.) You actually don't need to use
g_queue_new() — if you're into this kind of thing you can put a GQueue (not a
GQueue *) in your private structure — but no big deal if you're not. Much as I
like foreach functions, I think explicit iteration would be clearer to copy the
list into the queue, but again if you're not feeling keen don't worry about it
too much.


@@ -1558,7 +1558,7 @@ open_porter (WockyMetaPorter *self,
  * Make an asynchronous request to open a connection to @contact if
  * one is not already open. The reference count of the porter to
  * @contact will be incrememented and so after completion
- * wocky_meta_porter_unref() should be called on contact to release
+ * wocky_meta_porter_unhold() should be called on contact to release
  * the reference.

If I were a pedant I would ask for the documentation to not talk about
references, given that the functions no longer talk about references. But NBD.


> > + */
> > +WockyXmppConnection *
> > +wocky_connection_factory_make_connection_finish (
> > +    WockyConnectionFactory *self,
> > +    GAsyncResult *result,
> > +    GError **error)
> > +{
> > +  wocky_implement_finish_return_pointer (self,
> > +      wocky_connection_factory_make_connection_async);
> > +}
> > 
> > No, the GSimpleAsyncResult should own a ref on the WockyXmppConnection, and
> > this function should return a ref to it. It's completely legitimate for the
> > caller to never actually call _make_connection_finish(), in which case you'll
> > leak the connection. (I was wondering why you wanted the new macro.)
> 
> That's not quite right. The connection returned actually belongs to the
> connection factory. If you call make_connection_finish you get the connection,
> but if you don't ref it before unreffing the connection factory it will be
> lost. This is the exact same behaviour as the connector.
> 
> So, you want this changed? And in the connector too?

Yes. The GIO convention is that _finish() functions return new refs. (If
existing Wocky code gets this wrong, it's wrong too!) I think this would mean
you wouldn't need the new non-copying macro.

I'd call the connector functions wocky_ll_connector_new_incoming_async(),
wocky_ll_connector_new_outgoing_async(), wocky_ll_connector_new_finish().

> > What happens if an incoming connection arrives in the gap? A valid answer is
> > “it can't happen” :)
> 
> Yeah it can't happen. Contacts can only get your address once you've published
> it in the appropriate avahi record which is when you call set_jid.
> 
> > What happens if you change the JID after the loopback porter has been created?
> 
> A new loopback porter is created when set_jid is called. I'm not sure about
> this because in the worst case you call set_jid n times, there will be n extra
> loopback porters in the hash table (which will all be closed and cleaned up
> when the porter is closed). On the other hand, if it is processing stanzas,
> these could be lost if we bin the porter when changing JID. At the moment,
> nothing will be lost. What do you think?

As we discussed on IRC — maybe we should just document that you should only
call this once, and warn if it's called more than once. :)

Time to review the original patches I skipped…

-- 
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.