After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 584257 - Allow iterating over sockets associated with GSocketListener
Allow iterating over sockets associated with GSocketListener
Status: RESOLVED WONTFIX
Product: glib
Classification: Platform
Component: gio
2.21.x
Other All
: Normal enhancement
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2009-05-30 06:44 UTC by Ondrej Jirman
Modified: 2018-03-21 00:37 UTC
See Also:
GNOME target: ---
GNOME version: Unversioned Enhancement


Attachments
gsocketlistener: Add g_socket_listener_get_sockets() (2.85 KB, patch)
2014-06-17 09:12 UTC, Philip Withnall
none Details | Review
gsocketlistener: Add g_socket_listener_get_sockets() (2.91 KB, patch)
2017-10-17 15:19 UTC, Philip Withnall
reviewed Details | Review

Description Ondrej Jirman 2009-05-30 06:44:51 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);
Comment 1 Alexander Larsson 2009-06-15 13:55:13 UTC
Using callbacks seems a bit weird here. We should just add a getter like

GSocket ** g_socket_listener_get_sockets().
Comment 2 Philip Withnall 2014-06-17 09:12:08 UTC
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.
Comment 3 Michael Catanzaro 2017-10-17 14:49:36 UTC
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.
Comment 4 Philip Withnall 2017-10-17 15:19:09 UTC
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.
Comment 5 Philip Withnall 2017-10-17 15:20:15 UTC
(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. :-)
Comment 6 Michael Catanzaro 2017-10-17 19:17:15 UTC
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.
Comment 7 Philip Withnall 2017-10-18 23:07:50 UTC
(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.
Comment 8 Michael Catanzaro 2018-03-21 00:36:28 UTC
Adjusting component to gio in preparation for GitLab migration
Comment 9 Michael Catanzaro 2018-03-21 00:37:20 UTC
(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.