GNOME Bugzilla – Bug 584257
Allow iterating over sockets associated with GSocketListener
Last modified: 2018-03-21 00:37:20 UTC
I would like to be able to iterate over sockets associated with GSocketListener object, in order to be able to change some socket options (eg. turning off Nagle's algorithm, ...) When I add sockets to the GSocketListener using g_socket_listener_add_inet_port() or g_socket_listener_add_address(), then I'll not be able to access underlying GSocket in order to tune its parameters. Arguably, I can always use g_socket_listener_add_socket(), but then I would just be dupplicating what g_socket_listener_add_inet_port() does in my own code. Can we add something like this? I'd be willing to provide a patch. g_socket_listener_foreach_socket(GSocketListener* listener, GFunc* callback, gpointer user_data); So that the following is possible: void _cb(GSocket* sock, gpoitner user_data) { //XXX: tune socket parameters here } g_socket_listener_foreach_socket(listener, _cb, NULL);
Using callbacks seems a bit weird here. We should just add a getter like GSocket ** g_socket_listener_get_sockets().
Created attachment 278575 [details] [review] gsocketlistener: Add g_socket_listener_get_sockets() This exposes the raw GSockets in the listener to allow more esoteric methods to be called on them than is sensible to expose in the GSocketListener API.
Review of attachment 278575 [details] [review]: Better late than never.... Is there still an application that wants to use this? I'd hesitate to add new API unless it will actually be used by something in the near future. Other than that, it looks fine. ::: gio/gsocketlistener.c @@ +1134,3 @@ + * Returns: (transfer none) (element-type GSocket): an array of #GSockets + * + * Since: 2.42 It'll need to be updated. @@ +1137,3 @@ + */ +GPtrArray * +g_socket_listener_get_sockets (GSocketListener *listener) You have an extra space here. ::: gio/gsocketlistener.h @@ +146,3 @@ void g_socket_listener_close (GSocketListener *listener); +GLIB_AVAILABLE_IN_2_42 It'll need to be updated.
Created attachment 361751 [details] [review] gsocketlistener: Add g_socket_listener_get_sockets() This exposes the raw GSockets in the listener to allow more esoteric methods to be called on them than is sensible to expose in the GSocketListener API.
(In reply to Michael Catanzaro from comment #3) > Review of attachment 278575 [details] [review] [review]: > > Better late than never.... > > Is there still an application that wants to use this? I'd hesitate to add > new API unless it will actually be used by something in the near future. > Other than that, it looks fine. IIRC, I switched the application I originally wrote this patch for to use something other than GSocketListener. I think the patch is still generally useful though. You are free to disagree and reject the patch; I have no horse in this race. :-)
Review of attachment 361751 [details] [review]: I agree that it's generally-useful. And looking at the poor state of the tests in gio/tests/socket-listener.c, I don't think we need to add a new one for this API. So I think this patch is fine as-is. But I'll leave the status as reviewed for now, since in general I don't think it's a great idea to push new APIs unless there's at least one application that plans to use it. The patch is straightforward and should be simple to resurrect if somebody wants it in the future.
(In reply to Michael Catanzaro from comment #6) > But I'll leave the status as reviewed for now, since in general I don't > think it's a great idea to push new APIs unless there's at least one > application that plans to use it. The patch is straightforward and should be > simple to resurrect if somebody wants it in the future. I would prefer to close this bug, either having pushed the patches, or having rejected them. Let’s not leave bugs open with no clear route forwards for them.
Adjusting component to gio in preparation for GitLab migration
(In reply to Philip Withnall from comment #7) > I would prefer to close this bug, either having pushed the patches, or > having rejected them. Let’s not leave bugs open with no clear route forwards > for them. Yeah good point. In that case, these are rejected on the basis that no application plans to use them. But feel free to reopen if an app wants to use this.