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.
This commit is contained in:
Thomas Haller 2022-09-26 13:39:23 +02:00
parent 8ab23e5b2d
commit a1ab9d9e1c
No known key found for this signature in database
GPG key ID: 29C2366E4DFC5728
6 changed files with 65 additions and 12 deletions

View file

@ -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);
}

View file

@ -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;
}

View file

@ -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;

View file

@ -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);

View file

@ -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();
}

View file

@ -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,