[Bug 36110] Add util functions for determining socket address type and access control

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Mon Jun 13 17:39:15 CEST 2011


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

--- Comment #2 from Will Thompson <will.thompson at collabora.co.uk> 2011-06-13 08:39:14 PDT ---
+  // Find best socket address type

Please use /* C89 comments */ rather than // C99 comments (despite it being
2011).

This kind of comment belongs in a block above the function:

/*
 * _tp_determine_socket_address_type:
 *
 * Determines the best-available socket address type.
 */
TpSocketAddressType
_tp_determine_socket_address_type (GHashTable *supported_sockets,
    GError **error)

or similar.

I notice that you've actually moved these functions over almost verbatim from
telepathy-glib/stream-tube-channel.c but changed the commenting style and
removing the pre-function block comment from find_best_access_control() which
had slightly more information. :) I'd personally prefer patches that move code
to be joined together, to make it clear that the code's been moved, rather than
apparently being written from scratch and then later discovering that the same
code was removed from another file.

The branching in the code is a bit weird: both
_tp_determine_socket_address_type and _tp_determine_access_control_type have a
lot of duplication.

+  self->priv->socket_type = _tp_determine_socket_address_type (
+      supported_sockets, &error);
+  self->priv->access_control = _tp_determine_access_control_type (
+      supported_sockets, self->priv->socket_type, &error);

This is wrong: you can't pass 'error' to the second function without having
checked that the first function returned success.

Any particular reason for removing the more-specific DEBUG() output from the
error paths of create_client_socket? Previously the message told the developer
immediately at what stage the error occurred; now it does not.

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



More information about the telepathy-bugs mailing list