GNOME Bugzilla – Bug 731763
Add UDP support to GSocketListener
Last modified: 2018-05-24 16:40:07 UTC
It would be useful if GSocketListener supported UDP as well as TCP. How about something like: guint16 g_socket_listener_add_inet_port_full (GSocketListener *listener, GSocketType type, GSocketProtocol protocol, guint16 port, GObject *source_object, GError **error); which would generalise the current implementations of g_socket_listener_add_inet_port() and g_socket_listener_add_any_inet_port() (so it would allow @port to be zero). I’m wondering whether adding support for setting the default UDP destination address (i.e. calling g_socket_connect() on each socket) would be best supported in this function call, or in a separate function call. What do people think? I can come up with a patch if it’s likely to be accepted.
Hm... but what would it do? listen() and accept() don't work with UDP sockets. It might be useful to have some helper classes for UDP sockets, but I think they'd have to be separate from GSocketListener and GSocketClient because of the different semantics. And we'd want to see an example of it in use somewhere before adding it to GLib. (WONTFIXing, but feel free to continue discussion of what a UDP-helping API might look like, or even reopen and retitle the bug.)
(In reply to comment #1) > Hm... but what would it do? listen() and accept() don't work with UDP sockets. Well, the complicated bit is negotiating the port numbers with bind(), which is still applicable for UDP sockets even if listen() isn’t. The user’s code would then grab the sockets using g_socket_listener_get_sockets() (bug #584257) and create GSources for them, or start sending and receiving directly. > And we'd want to see an example of it in use somewhere > before adding it to GLib. Agreed. I guess I hoped this would be a discussion bug before it turned into a patch bug. :-)
Maybe what we want is a function to create/bind n GSockets based on a GSocketAddress (so basically the useful part of these proposed patches), so basically splitting out the interesting part of GSocketListener into a helper function.
As Olivier says, adding this to GSocketListener is a whole lot less useful without the GDatagramStream stuff rejected in bug #697907, since we can’t get a GSocketConnection for an incoming UDP packet ‘stream’. I’ve split out the useful IPv4/IPv6 binding code into g_socket_bind_inet_port() and rebased the existing GSocketListener methods on this.
Created attachment 279402 [details] [review] gsocket: Add g_socket_bind_inet_port() and use in GSocketListener The core functionality of • g_socket_listener_add_inet_port(), and • g_socket_listener_add_any_inet_port() is to attempt binding IPv6 and IPv4 ports and to return a suitably bound and working socket if possible. This functionality would be useful for types of socket other than streams — for example, when creating a datagram-based server using UDP. As well as adding the new g_socket_bind_inet_port() implementation (adapted from the existing GSocketListener implementations), this ports GSocketListener to use it. New API: • g_socket_bind_inet_port()
Created attachment 279654 [details] [review] gsocket: Add g_socket_bind_inet_port() and use in GSocketListener The core functionality of • g_socket_listener_add_inet_port(), and • g_socket_listener_add_any_inet_port() is to attempt binding IPv6 and IPv4 ports and to return a suitably bound and working socket if possible. This functionality would be useful for types of socket other than streams — for example, when creating a datagram-based server using UDP. As well as adding the new g_socket_bind_inet_port() implementation (adapted from the existing GSocketListener implementations), this ports GSocketListener to use it. New API: • g_socket_bind_inet_port()
Comment on attachment 279654 [details] [review] gsocket: Add g_socket_bind_inet_port() and use in GSocketListener >+ * g_socket_bind_inet_port: Needs a better name; that doesn't at all imply that it creates a new GSocket (let alone *2* new sockets). This isn't really a core GSocket operation, so maybe something like "g_socket_util_bind_inet()"? Maybe even keep it in gsocketlistener.c? Not sure exactly what you'd call it in that case. >+ * @any_address: %TRUE to bind to any interface, %FALSE to bind to loopback >+ * interfaces only >+ * @candidate_port: (inout): location of the candidate port to attempt to bind >+ * to, and the return location for the chosen port on success That's clunky, and insufficient anyway; what if you want to bind a single specific IP address other than loopback? It should take a GInetSocketAddress and an (out) port (or else a GInetAddress and an (inout) port, but I don't really like inout args...) Or maybe have multiple public convenience APIs. >+ * Create a new inet #GSocket (i.e. an IPv4 or IPv6 socket) with the given @type "one or more new IP #GSockets" >+GPtrArray * GList would be more "glib-like".
Created attachment 288723 [details] [review] gsocket: Add g_socket_bind_inet_port() and use in GSocketListener The core functionality of • g_socket_listener_add_inet_port(), and • g_socket_listener_add_any_inet_port() is to attempt binding IPv6 and IPv4 ports and to return a suitably bound and working socket if possible. This functionality would be useful for types of socket other than streams — for example, when creating a datagram-based server using UDP. As well as adding the new g_socket_bind_inet_port() implementation (adapted from the existing GSocketListener implementations), this ports GSocketListener to use it. New API: • g_socket_bind_inet_port()
(In reply to comment #7) > (From update of attachment 279654 [details] [review]) > >+ * g_socket_bind_inet_port: > > Needs a better name; that doesn't at all imply that it creates a new GSocket > (let alone *2* new sockets). > > This isn't really a core GSocket operation, so maybe something like > "g_socket_util_bind_inet()"? Changed. > >+ * @any_address: %TRUE to bind to any interface, %FALSE to bind to loopback > >+ * interfaces only > >+ * @candidate_port: (inout): location of the candidate port to attempt to bind > >+ * to, and the return location for the chosen port on success > > That's clunky, and insufficient anyway; what if you want to bind a single > specific IP address other than loopback? It should take a GInetSocketAddress > and an (out) port (or else a GInetAddress and an (inout) port, but I don't > really like inout args...) Or maybe have multiple public convenience APIs. I agree it’s clunky, but I can’t suggest anything better, due to the need to create new GInetAddresses within the function so that both IPv6 and IPv4 sockets can be tried. I’m not sure there’s any way to copy a GInetAddress and change its socket family correctly in all cases: • If the GInetAddress was originally created with *_new_any() or *_new_loopback(), it’s fine. • If it was originally created with *_new_from_bytes(), the binary address is only good for one socket family. • If it was originally created with *_new_from_string(), the string encodes the socket family and the family can’t be specified in subsequent calls to the constructor. So the function can only work in the *_new_any() and *_new_loopback() cases anyway. Hence the boolean. :-( > >+ * Create a new inet #GSocket (i.e. an IPv4 or IPv6 socket) with the given @type > > "one or more new IP #GSockets" Fixed. > >+GPtrArray * > > GList would be more "glib-like". I would rather avoid GLists — GPtrArray has nice things like a free_func while the use of owned GObjects inside GLists is pretty much guaranteed to get leaked by someone, somewhere. Sorry for the slow reply to this one.
Review of attachment 288723 [details] [review]: This should be tested in gio/tests/socket.c. ::: gio/gsocket.c @@ +4662,3 @@ +/** + * g_socket_util_bind_inet: I think it's very confusing for a function named g_socket_util_bind_inet() to create an unknown number of sockets and return those, rather than simply bind an address to an existing socket. I understand that it would be useful to move this functionality into a new API. Perhaps I'm only really complaining about the name. A terrible strawman name proposal: g_socket_create_bound_sockets(). I'm sure we can come up with much better than that, though. I also wondered why the higher-level GSocketConnection API would not be more suitable for a convenience function like this, but I guess the problem is that a GSocketConnection is a GIOStream, which doesn't really map well to datagrams.
Adjusting component to gio in preparation for GitLab migration
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/891.