[Bug 29218] TpStreamTube - high level stream tube API

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Tue Aug 10 19:28:34 CEST 2010


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

--- Comment #8 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-08-10 10:28:33 PDT ---
Some quick comments:

TpStreamTube is missing documentation (at the moment make check will fail), but
I'm sure you knew that already.

If we don't drop it, TpChannelView needs documentation too.

> +#define GET_PRIV(o) (((TpChannelView *)o)->priv)

I really dislike this idiom; it takes the type-safety you could have had by
using self->priv (see TpBaseProtocol, for instance) and throws it away by
casting o without a check, while being harder to type than self->priv.

TpStreamTubeClass, TpChannelViewClass need ABI padding.

The GAsyncResult idiom is that tp_stream_tube_offer_existing_async should have
its own _finish function, I think?

determine_socket_type() looks as though it should check for an access control
type we understand, although we should probably declare that (IPv4, Localhost)
is mandatory-to-implement, and if you support Unix sockets then you must
support (Unix, Localhost).

It'd probably be better for this code to prefer abstract Unix sockets on
platforms that have them (Linux), and to avoid DoS/connection-stealing on
multi-user systems, you should prefer Port (for IPv4) and Credentials (for
Unix). Ability to test these is blocked by Bug #12999, though.

> +      g_set_error (error, TP_ERRORS,
> +          TP_ERROR_RESOURCE_UNAVAILABLE,
> +          "No supported socket types");

NotImplemented?

Why does _channel_accepted call determine_socket_type() again? Surely the tube
should remember which socket type (and access control mechanism) it used?

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



More information about the telepathy-bugs mailing list