GNOME Bugzilla – Bug 737116
Add functions to print GSocketConnectables and addresses as strings
Last modified: 2015-10-13 14:53:09 UTC
For debugging and presenting addresses to the user, it would be useful to have functions like: 1. gchar *g_network_address_to_string (GNetworkAddress *addr); 2. gchar *g_socket_address_to_string (GSocketAddress *addr); 3. gchar *g_socket_connectable_to_string (GSocketConnectable *addr); printing things like: 1. [scheme:]hostname[:port] 2. address[%scope][:port] (for GInetSocketAddresses) 3. Appropriate *_to_string() output from the implementation type I can provide a patch if these would be useful.
sure
Created attachment 288698 [details] [review] gio: Add *_to_string() methods for various network addresses Add string serialisation functions for GNetworkAddress, GSocketAddress and GSocketConnectable. These are intended for use in UIs and debug output, and not for serialisation in network or disc protocols.
Created attachment 288701 [details] [review] gio: Add *_to_string() methods for various network addresses Add string serialisation functions for GNetworkAddress, GSocketAddress and GSocketConnectable. These are intended for use in UIs and debug output, and not for serialisation in network or disc protocols.
Created attachment 312278 [details] [review] gio: Add *_to_string() methods for various network addresses Add string serialisation functions for GNetworkAddress, GSocketAddress and GSocketConnectable. These are intended for use in UIs and debug output, and not for serialisation in network or disc protocols.
Comment on attachment 312278 [details] [review] gio: Add *_to_string() methods for various network addresses >+ * Format a #GNetworkAddress as a string. This is a human-readable format for >+ * use in interfaces or debugging output, and is not a stable serialization >+ * format. If it's for use in interfaces you should document what it looks like... but I think the two use cases are incompatible anyway. For UIs, you want something that looks nice and matches the user's mental model of the situation. (Eg, in a web browser, you don't want to include "http" and ":80" in the string, since that's understood.) For debugging, you want something that includes as much information as possible. (Eg, it might be nice if g_network_address_to_string()'s output included the addresses it resolves to as well, if they've been resolved already.) Which use case actually inspired you to write the patch? >+ * Since: 2.47.0 2.48. We don't use unstable version numbers in "Since". >+ scheme = g_network_address_get_scheme (addr); >+ if (scheme != NULL) >+ { >+ g_string_append_printf (out, "%s:", scheme); >+ } (Don't use {}s on single-line bodies.) If you're going to include the scheme then probably you should format the string as a URL... >+g_socket_address_to_string (GSocketAddress *addr) >+ /* Scope ID (IPv6 only). */ >+ if (g_inet_address_get_family (a) == G_SOCKET_FAMILY_IPV6) >+ { >+ g_string_append_printf (out, "%%%u", >+ g_inet_socket_address_get_scope_id (sa)); Should only add that if it's non-0 >+ if (port != 0) >+ { >+ g_string_append_printf (out, ":%u", port); >+ } in the IPv6 case it would probably be best to do this URL-style and wrap "[]" around the address when there's a port, to distinguish the port's colon+number from the address's colons+numbers. >+ else >+ { >+ /* Unsupported subclass of #GSocketAddress. */ >+ g_assert_not_reached (); Um, no? If the function explicitly only works for GInetSocketAddress, then it should be a GInetSocketAddress function, not GSocketAddress. >+g_socket_connectable_to_string (GSocketConnectable *addr) >+{ >+ g_return_val_if_fail (G_IS_SOCKET_CONNECTABLE (addr), NULL); >+ >+ if (G_IS_SOCKET_ADDRESS (addr)) GSocketConnectable is an interface, so it can be extended even though it doesn't have placeholder fields. So this could be a real method.
Created attachment 312638 [details] [review] • Substantially rewritten. • Formatting suggestions applied. • Stupid g_assert_not_reached() removed. • Moved to a vfunc of GSocketConnectable. • Implemented in GUnixSocketAddress and GNetworkService. • Some (not exhaustive) tests added. • Clarified that my original use case was debugging, not UIs. --- gsocketconnectable: Add a to_string() virtual method Add string serialisation functions for GNetworkAddress, GSocketAddress, GUnixSocketAddress, GInetSocketAddress, GNetworkService and GSocketConnectable. These are intended for use in debug output, not for serialisation in network or disc protocols. They are implemented as a new virtual method on GSocketConnectable: g_socket_connectable_to_string(). GInetSocketAddress and GUnixSocketAddress now implement GSocketConnectable directly to implement to_string(). Previously they implemented it via their abstract parent class, GSocketAddress.
Comment on attachment 312638 [details] [review] • Substantially rewritten. Awesome. Just one suggestion: >+ * Returns: (transfer full) (nullable): the formatted string, or %NULL if the >+ * type of #GSocketConnectable does not support it You could return "g_strdup (G_OBJECT_TYPE_NAME (connectable))" as a default instead. It's not super-useful debugging-wise, but at least then callers don't have to deal with possibly getting back NULL.
Pushed with the suggested change to make g_socket_connectable_to_string() non-nullable. Thanks.