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 737116 - Add functions to print GSocketConnectables and addresses as strings
Add functions to print GSocketConnectables and addresses as strings
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: network
2.41.x
Other All
: Normal enhancement
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2014-09-22 13:23 UTC by Philip Withnall
Modified: 2015-10-13 14:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gio: Add *_to_string() methods for various network addresses (7.17 KB, patch)
2014-10-16 16:10 UTC, Philip Withnall
none Details | Review
gio: Add *_to_string() methods for various network addresses (7.32 KB, patch)
2014-10-16 16:30 UTC, Philip Withnall
none Details | Review
gio: Add *_to_string() methods for various network addresses (7.52 KB, patch)
2015-09-28 12:10 UTC, Philip Withnall
needs-work Details | Review
• Substantially rewritten. (19.83 KB, patch)
2015-10-04 14:41 UTC, Philip Withnall
committed Details | Review

Description Philip Withnall 2014-09-22 13:23:57 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.
Comment 1 Dan Winship 2014-10-05 14:47:44 UTC
sure
Comment 2 Philip Withnall 2014-10-16 16:10:36 UTC
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.
Comment 3 Philip Withnall 2014-10-16 16:30:10 UTC
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.
Comment 4 Philip Withnall 2015-09-28 12:10:33 UTC
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 5 Dan Winship 2015-10-03 13:43:28 UTC
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.
Comment 6 Philip Withnall 2015-10-04 14:41:46 UTC
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 7 Dan Winship 2015-10-10 12:06:10 UTC
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.
Comment 8 Philip Withnall 2015-10-13 14:53:04 UTC
Pushed with the suggested change to make g_socket_connectable_to_string() non-nullable. Thanks.