From https://bugs.freedesktop.org/show_bug.cgi?id=31376#c19: """ There are some util functions, some moved from stream tube so they can be shared, and some new, that can be merged right away. Those can be merged from this branch: http://cgit.freedesktop.org/~sjokkis/telepathy-glib/log/?h=util """
Here is my version http://cgit.collabora.com/git/user/cassidy/telepathy-glib/log/?h=socket-util-38997 It's based on Morten's branch with the following changes: - rebased on top of master - I squashed the "create new utilty function" and "use it in TpStreamTube" commits together. This makes reviewing easier and we can easily see if the new function is just a factoring out of existing code. - _tp_determine_socket_address_type() and _tp_determine_access_control_type() are now statics as they weren't used outside of util.c - _tp_set_socket_address_type_and_access_control_type() now directly pass the GError from the caller rather than using its own and use g_propagate_error(). - I re-ordered the arguments of _tp_create_local_socket so the (out) args are last. - _tp_create_local_socket(): set unix_address to NULL if we create an Inet socket. - Some coding style fixes.
Created attachment 48809 [details] [review] Factor out _tp_set_socket_address_type_and_access_control_type() and _tp_create_client_socket to util-internal
Created attachment 48810 [details] [review] Factor out _tp_create_local_socket() to tests/lib/util
Created attachment 48811 [details] [review] Factor out _tp_destroy_socket_control_list() to tests/lib/util
Shouldn't you protect all gunix* with #ifdef for _tp_create_local_socket()? I think even the +#include <gio/gunixsocketaddress.h> should be protected because those headers are not installed on windows'glib. I could be wrong though... In all those functions you assume out args are not NULL, it's fine since they are internal APIs, but maybe add a g_assert on top of the methods to make it explicit? The rest seems OK.
(In reply to comment #5) > Shouldn't you protect all gunix* with #ifdef for _tp_create_local_socket()? I > think even the +#include <gio/gunixsocketaddress.h> should be protected because > those headers are not installed on windows'glib. I could be wrong though... You're right. Fixed (amended in 2nd commit and added a new one). While I was at it, I fixed the stream tube test to pass if UNIX sockets are not there. > In all those functions you assume out args are not NULL, it's fine since they > are internal APIs, but maybe add a g_assert on top of the methods to make it > explicit? Good idea; done (amended)
Created attachment 49001 [details] [review] Factor out _tp_set_socket_address_type_and_access_control_type() and _tp_create_client_socket to util-internal
Created attachment 49002 [details] [review] Factor out _tp_create_local_socket() to tests/lib/util
Created attachment 49003 [details] [review] Factor out _tp_destroy_socket_control_list() to tests/lib/util
Created attachment 49004 [details] [review] tests/lib/stream-tube-chan: don't try to use UNIX socket if not supported
Created attachment 49005 [details] [review] gnio-util: properly set the GError if UNIX sockets are not implemented
Created attachment 49006 [details] [review] test-stream-tube: pass if UNIX sockets are not implemented
I backported the fix that should be merged into stable first: http://cgit.collabora.com/git/user/cassidy/telepathy-glib/log/?h=unix-sockets
Reviewing only the stable-branch fixes: + g_assert (FALSE); g_assert_not_reached, surely? The rest of the unix-sockets branch looks fine.
(In reply to comment #13) > I backported the fix that should be merged into stable first: > http://cgit.collabora.com/git/user/cassidy/telepathy-glib/log/?h=unix-sockets Merged, so http://cgit.collabora.com/git/user/cassidy/telepathy-glib/log/?h=socket-util-38997 is the last thing needing review.
Merged to master; will be in 0.15.5.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.