libnm: use NMRefString for nm_connection_get_path()

NMConnection is an interface, implemented by NMSimpleConnection and
NMRemoteConnection.

For the most part, an NMConnection is only the content of the profile
(the settings). The "path" of the connection refers to the D-Bus path,
and wouldn't really make sense of the NMConnection interface or the
NMSimpleConnection type.

As such, the daemon (which only uses NMConnection and
NMSimpleConnection) never sets the path. Only libnm does.

NMClient uses NMRefString extensively for the D-Bus interface and the
path is already internalized. Take advantage of that. It is very likely,
that we are able to share the path instance in libnm at which point it
makes sense to use NMRefString.

Also, during nm_simple_connection_new_clone(), we can just take another
reference instead of cloning the string.

6 files changed, 65 insertions, 12 deletions

diff --git a/src/libnm-client-impl/nm-remote-connection.c b/src/libnm-client-impl/nm-remote-connection.c
index 23dfc1dac3..0810e540dc 100644
--- a/src/libnm-client-impl/nm-remote-connection.c
+++ b/src/libnm-client-impl/nm-remote-connection.c
@@ -696,7 +696,7 @@ static void
 register_client(NMObject *nmobj, NMClient *client, NMLDBusObject *dbobj)
 {
     NM_OBJECT_CLASS(nm_remote_connection_parent_class)->register_client(nmobj, client, dbobj);
-    nm_connection_set_path(NM_CONNECTION(nmobj), dbobj->dbus_path->str);
+    _nm_connection_set_path_rstr(NM_CONNECTION(nmobj), dbobj->dbus_path);
     _nm_client_get_settings_call(client, dbobj);
 }
 
diff --git a/src/libnm-core-impl/nm-connection.c b/src/libnm-core-impl/nm-connection.c
index 60e7eb7ff2..6c73c66bb5 100644
--- a/src/libnm-core-impl/nm-connection.c
+++ b/src/libnm-core-impl/nm-connection.c
@@ -11,6 +11,7 @@
 #include <arpa/inet.h>
 
 #include "libnm-glib-aux/nm-uuid.h"
+#include "libnm-glib-aux/nm-ref-string.h"
 #include "nm-connection-private.h"
 #include "nm-utils.h"
 #include "nm-setting-private.h"
@@ -59,7 +60,7 @@ _nm_connection_private_clear(NMConnectionPrivate *priv)
 {
     if (priv->self) {
         _nm_connection_clear_settings(priv->self, priv);
-        nm_clear_g_free(&priv->path);
+        nm_clear_pointer(&priv->path, nm_ref_string_unref);
         priv->self = NULL;
     }
 }
@@ -2913,14 +2914,15 @@ nm_connection_dump(NMConnection *connection)
 void
 nm_connection_set_path(NMConnection *connection, const char *path)
 {
-    NMConnectionPrivate *priv;
-
     g_return_if_fail(NM_IS_CONNECTION(connection));
 
-    priv = NM_CONNECTION_GET_PRIVATE(connection);
+    nm_ref_string_reset_str(&NM_CONNECTION_GET_PRIVATE(connection)->path, path);
+}
 
-    g_free(priv->path);
-    priv->path = g_strdup(path);
+void
+_nm_connection_set_path_rstr(NMConnection *connection, NMRefString *path)
+{
+    nm_ref_string_reset(&NM_CONNECTION_GET_PRIVATE(connection)->path, path);
 }
 
 /**
@@ -2937,6 +2939,12 @@ nm_connection_get_path(NMConnection *connection)
 {
     g_return_val_if_fail(NM_IS_CONNECTION(connection), NULL);
 
+    return nm_ref_string_get_str(NM_CONNECTION_GET_PRIVATE(connection)->path);
+}
+
+NMRefString *
+_nm_connection_get_path_rstr(NMConnection *connection)
+{
     return NM_CONNECTION_GET_PRIVATE(connection)->path;
 }
 
diff --git a/src/libnm-core-impl/nm-setting-private.h b/src/libnm-core-impl/nm-setting-private.h
index 005d04808a..221a527e12 100644
--- a/src/libnm-core-impl/nm-setting-private.h
+++ b/src/libnm-core-impl/nm-setting-private.h
@@ -20,13 +20,15 @@
 
 /*****************************************************************************/
 
+struct _NMRefString;
+
 typedef struct {
     NMConnection *self;
 
     NMSetting *settings[_NM_META_SETTING_TYPE_NUM];
 
     /* D-Bus path of the connection, if any */
-    char *path;
+    struct _NMRefString *path;
 } NMConnectionPrivate;
 
 extern GTypeClass *_nm_simple_connection_class_instance;
diff --git a/src/libnm-core-impl/nm-simple-connection.c b/src/libnm-core-impl/nm-simple-connection.c
index 802be3caa4..a9c66dc72d 100644
--- a/src/libnm-core-impl/nm-simple-connection.c
+++ b/src/libnm-core-impl/nm-simple-connection.c
@@ -136,15 +136,12 @@ NMConnection *
 nm_simple_connection_new_clone(NMConnection *connection)
 {
     NMConnection *clone;
-    const char   *path;
 
     g_return_val_if_fail(NM_IS_CONNECTION(connection), NULL);
 
     clone = nm_simple_connection_new();
 
-    path = nm_connection_get_path(connection);
-    if (path)
-        nm_connection_set_path(clone, path);
+    _nm_connection_set_path_rstr(clone, _nm_connection_get_path_rstr(connection));
 
     nm_connection_replace_settings_from_connection(clone, connection);
 
diff --git a/src/libnm-core-impl/tests/test-general.c b/src/libnm-core-impl/tests/test-general.c
index d66b820d7f..904d89f337 100644
--- a/src/libnm-core-impl/tests/test-general.c
+++ b/src/libnm-core-impl/tests/test-general.c
@@ -11081,6 +11081,43 @@ test_direct_string_is_refstr(void)
 
 /*****************************************************************************/
 
+static void
+test_connection_path(void)
+{
+    gs_unref_object NMConnection *conn = NULL;
+    const char *const             PATH = "/org/freedesktop/NetworkManager/Settings/171950003017";
+    const char                   *path;
+    NMRefString                  *rstr;
+
+    g_assert(!nmtst_ref_string_find(PATH));
+
+    conn = nmtst_create_minimal_connection("test_setting_ip6_gateway",
+                                           NULL,
+                                           NM_SETTING_WIRED_SETTING_NAME,
+                                           NULL);
+
+    g_assert(!nm_connection_get_path(conn));
+    g_assert(!nmtst_ref_string_find(PATH));
+
+    nm_connection_set_path(conn, PATH);
+
+    path = nm_connection_get_path(conn);
+    g_assert_cmpstr(path, ==, PATH);
+
+    /* nm_connection_get_path() gives a NMRefString. This is an
+     * implementation detail, but libnm (which statically links with
+     * libnm-core) may choose to rely on that. */
+    rstr = nmtst_ref_string_find(PATH);
+    g_assert(rstr);
+    g_assert(NM_REF_STRING_UPCAST(path) == rstr);
+
+    g_clear_object(&conn);
+
+    g_assert(!nmtst_ref_string_find(PATH));
+}
+
+/*****************************************************************************/
+
 NMTST_DEFINE();
 
 int
@@ -11426,6 +11463,7 @@ main(int argc, char **argv)
 
     g_test_add_func("/core/general/test_system_encodings", test_system_encodings);
     g_test_add_func("/core/general/test_direct_string_is_refstr", test_direct_string_is_refstr);
+    g_test_add_func("/core/general/test_connection_path", test_connection_path);
 
     return g_test_run();
 }
diff --git a/src/libnm-core-intern/nm-core-internal.h b/src/libnm-core-intern/nm-core-internal.h
index 4e1bab4723..9e9a673a10 100644
--- a/src/libnm-core-intern/nm-core-internal.h
+++ b/src/libnm-core-intern/nm-core-internal.h
@@ -481,6 +481,14 @@ NMSettingIPConfig *nm_connection_get_setting_ip_config(NMConnection *connection,
 
 /*****************************************************************************/
 
+struct _NMRefString;
+
+void _nm_connection_set_path_rstr(NMConnection *connection, struct _NMRefString *path);
+
+struct _NMRefString *_nm_connection_get_path_rstr(NMConnection *connection);
+
+/*****************************************************************************/
+
 typedef enum {
     NM_BOND_OPTION_TYPE_INT,
     NM_BOND_OPTION_TYPE_BOTH,