[Bug 29218] TpStreamTube - high level stream tube API

[bugzilla-daemon at freedesktop.org]

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

--- Comment #32 from Guillaume Desmottes <guillaume.desmottes at collabora.co.uk> 2010-10-04 06:46:42 PDT ---
(In reply to comment #30)
> The stuff in gnio-util.c depends on GUnixConnection, which makes it troublesome
> to put in our API (GUnixConnection only conditionally exists).
> 
> The functions should be underscore-prefixed and in gnio-util-internal.h, or a
> new unix-internal.h; they should only be defined if HAVE_GIO_UNIX.
> 
> If they're needed by something outside telepathy-glib, we'll have to introduce
> a wrapper function which always exists, like this:

I added a wrapper as these functions are used by tests as well
(stream-tube-chan.c).

(In reply to comment #31)
> The general shape of the API looks fine. 

Cool, I will start documenting it then.

> > +static void
> > +tp_stream_tube_channel_constructed (GObject *obj)
> 
> I noticed this doesn't chain up. It should do that first.

done.

> > +   * @stream: (transfer none): the #TpStreamTubeConnection for the connection
> 
> I don't think (transfer) is applicable to objects in signals: the signal
> machinery always takes one ref for the duration of the signal emission, and the
> recipient needs to ref it if they want to keep it. It's worth mentioning it
> explicitly in the text though, like you did.

annotation removed.

> > +  if (n_failed > 0)
> > +    {
> > +      DEBUG ("Failed to prepare TpContact (unspecified error)");
> 
> If error == NULL and n_failed > 0, that unambiguously means the failed handles
> failed with InvalidHandle. Improved docs wording would be welcome.

fixed.

> > +        /* FIXME: set the address of the socket. Gio doesn't seem to have API
> > +         * to get the port before connecting without specifying the whole
> > +         * adress (we can't as we don't know which ports are available). */
> 
> You have to do a GIO-flavoured version of this pseudocode:
> 
> sock = socket (AF_INET, SOCK_STREAM, 0);
> bind (sock, addr={ AF_INET, sin_port=0, sin_addr=IN_ADDR_ANY }, addr_len);
> getsockname (sock, &real_addr, real_addr_len);
> /* time passes */
> connect (sock, blah blah blah);
> 
> I think the GIO version of that might be to make the GSocketClient earlier,
> call g_socket_client_set_local_address() for IN_ADDR_ANY with port 0, then call
> g_socket_client_get_local_address() to find out what the kernel actually gave
> us?

Unfortunatelly that doesn't work :( I opened
https://bugzilla.gnome.org/show_bug.cgi?id=631316 about this issue.

> > +      /* Pass the ref on tube_conn to the callback */
> > +
> > +      tp_connection_get_contacts_by_handle (
> > +          tp_channel_borrow_connection (TP_CHANNEL (self)),
> > +          1, &handle, 0, NULL,
> > +          _new_remote_connection_with_contact,
> > +          tube_conn, NULL, G_OBJECT (self));
> 
> You can't rely on the callback being called if you pass a weak object - if
> @self dies, the callback won't happen. If you just want to unref tube_conn in
> the "@self died" case, you can pass g_object_unref as the destructor here
> instead of releasing the ref in the callback?

fixed.

> > +  g_assert (self->priv->connection != NULL);
> 
> TP_IS_CONNECTION?

G_IS_SOCKET_CONNECTION actually :)

> > + * tp_stream_tube_connection_get_connection: (skip)
> 
> I wonder whether tp_stream_tube_connection_get_socket() would be better, given
> that TpConnection is a prominent part of our API? :-)

I renamed it to socket-connection to avoid confusion with TpConnection and
GSocket.

> _tp_create_temp_unix_socket looks like an ideal candidate for putting in GIO.

I opened https://bugzilla.gnome.org/show_bug.cgi?id=631314

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