core: avoid clone of all-connections list for nm_utils_complete_generic()

NMSettings exposes a cached list of all connection. We don't need
to clone it. Note that this is not save against concurrent modification,
meaning, add/remove of connections in NMSettings will invalidate the
list.

However, it wasn't save against that previously either, because
altough we cloned the container (GSList), we didn't take an additional
reference to the elements.

This is purely a performance optimization, we don't need to clone the
list. Also, since the original list is of type "NMConnection *const*",
use that type insistently, instead of dependent API requiring GSList.

IMO, GSList is anyway not a very nice API for many use cases because
it requires an additional slice allocation for each element. It's
slower, and often less convenient to use.
This commit is contained in:
Thomas Haller 2018-03-14 08:57:42 +01:00
parent f063ab41e9
commit e17cd1d742
25 changed files with 66 additions and 86 deletions

View file

@ -67,43 +67,47 @@ nm_utils_get_shared_wifi_permission (NMConnection *connection)
/*****************************************************************************/
static char *
get_new_connection_name (const GSList *existing_connections,
get_new_connection_name (NMConnection *const*existing_connections,
const char *preferred,
const char *fallback_prefix)
{
GSList *names = NULL;
const GSList *iter;
guint i;
gs_free const char **existing_names = NULL;
guint i, existing_len = 0;
g_assert (fallback_prefix);
for (iter = existing_connections; iter; iter = g_slist_next (iter)) {
NMConnection *candidate = NM_CONNECTION (iter->data);
const char *id;
if (existing_connections) {
existing_len = NM_PTRARRAY_LEN (existing_connections);
existing_names = g_new (const char *, existing_len);
for (i = 0; i < existing_len; i++) {
NMConnection *candidate;
const char *id;
id = nm_connection_get_id (candidate);
nm_assert (id);
candidate = existing_connections[i];
nm_assert (NM_IS_CONNECTION (candidate));
names = g_slist_append (names, (gpointer) id);
id = nm_connection_get_id (candidate);
nm_assert (id);
if ( preferred
&& nm_streq (preferred, id)) {
/* the preferred name is already taken. Forget about it. */
preferred = NULL;
existing_names[i] = id;
if ( preferred
&& nm_streq (preferred, id)) {
/* the preferred name is already taken. Forget about it. */
preferred = NULL;
}
}
nm_assert (!existing_connections[i]);
}
/* Return the preferred name if it was unique */
if (preferred) {
g_slist_free (names);
if (preferred)
return g_strdup (preferred);
}
/* Otherwise find the next available unique connection name using the given
* connection name template.
*/
for (i = 1; TRUE; i++) {
gboolean found = FALSE;
char *temp;
/* Translators: the first %s is a prefix for the connection id, such
@ -112,53 +116,44 @@ get_new_connection_name (const GSList *existing_connections,
* connection id. */
temp = g_strdup_printf (C_("connection id fallback", "%s %u"),
fallback_prefix, i);
for (iter = names; iter; iter = g_slist_next (iter)) {
if (nm_streq (iter->data, temp)) {
found = TRUE;
break;
}
}
if (!found) {
g_slist_free (names);
if (nm_utils_strv_find_first ((char **) existing_names,
existing_len,
temp) < 0)
return temp;
}
g_free (temp);
}
}
static char *
get_new_connection_ifname (NMPlatform *platform,
const GSList *existing_connections,
NMConnection *const*existing_connections,
const char *prefix)
{
guint i;
char *name;
const GSList *iter;
gboolean found;
guint i, j;
for (i = 0; TRUE; i++) {
char *name;
name = g_strdup_printf ("%s%d", prefix, i);
if (nm_platform_link_get_by_ifname (platform, name))
goto next;
for (iter = existing_connections, found = FALSE; iter; iter = g_slist_next (iter)) {
NMConnection *candidate = iter->data;
if (nm_streq0 (nm_connection_get_interface_name (candidate), name)) {
found = TRUE;
break;
if (existing_connections) {
for (j = 0; existing_connections[j]; j++) {
if (nm_streq0 (nm_connection_get_interface_name (existing_connections[j]),
name))
goto next;
}
}
if (!found)
return name;
return name;
next:
next:
g_free (name);
}
return NULL;
}
const char *
@ -250,7 +245,7 @@ void
nm_utils_complete_generic (NMPlatform *platform,
NMConnection *connection,
const char *ctype,
const GSList *existing_connections,
NMConnection *const*existing_connections,
const char *preferred_id,
const char *fallback_id_prefix,
const char *ifname_prefix,

View file

@ -31,7 +31,7 @@ const char *nm_utils_get_shared_wifi_permission (NMConnection *connection);
void nm_utils_complete_generic (NMPlatform *platform,
NMConnection *connection,
const char *ctype,
const GSList *existing_connections,
NMConnection *const*existing_connections,
const char *preferred_id,
const char *fallback_id_prefix,
const char *ifname_prefix,

View file

@ -114,7 +114,7 @@ static gboolean
complete_connection (NMDevice *device,
NMConnection *connection,
const char *specific_object,
const GSList *existing_connections,
NMConnection *const*existing_connections,
GError **error)
{
NMSettingAdsl *s_adsl;

View file

@ -215,7 +215,7 @@ static gboolean
complete_connection (NMDevice *device,
NMConnection *connection,
const char *specific_object,
const GSList *existing_connections,
NMConnection *const*existing_connections,
GError **error)
{
NMDeviceBtPrivate *priv = NM_DEVICE_BT_GET_PRIVATE ((NMDeviceBt *) device);

View file

@ -76,7 +76,7 @@ static gboolean
complete_connection (NMDevice *device,
NMConnection *connection,
const char *specific_object,
const GSList *existing_connections,
NMConnection *const*existing_connections,
GError **error)
{
NMSettingBond *s_bond;

View file

@ -115,7 +115,7 @@ static gboolean
complete_connection (NMDevice *device,
NMConnection *connection,
const char *specific_object,
const GSList *existing_connections,
NMConnection *const*existing_connections,
GError **error)
{
NMSettingBridge *s_bridge;

View file

@ -55,7 +55,7 @@ static gboolean
complete_connection (NMDevice *device,
NMConnection *connection,
const char *specific_object,
const GSList *existing_connections,
NMConnection *const*existing_connections,
GError **error)
{
NMSettingDummy *s_dummy;

View file

@ -1371,7 +1371,7 @@ static gboolean
complete_connection (NMDevice *device,
NMConnection *connection,
const char *specific_object,
const GSList *existing_connections,
NMConnection *const*existing_connections,
GError **error)
{
NMSettingWired *s_wired;

View file

@ -175,7 +175,7 @@ static gboolean
complete_connection (NMDevice *device,
NMConnection *connection,
const char *specific_object,
const GSList *existing_connections,
NMConnection *const*existing_connections,
GError **error)
{
NMSettingInfiniband *s_infiniband;

View file

@ -358,7 +358,7 @@ static gboolean
complete_connection (NMDevice *device,
NMConnection *connection,
const char *specific_object,
const GSList *existing_connections,
NMConnection *const*existing_connections,
GError **error)
{
NMSettingIPTunnel *s_ip_tunnel;

View file

@ -334,7 +334,7 @@ static gboolean
complete_connection (NMDevice *device,
NMConnection *connection,
const char *specific_object,
const GSList *existing_connections,
NMConnection *const*existing_connections,
GError **error)
{
NMSettingMacvlan *s_macvlan;

View file

@ -145,7 +145,7 @@ static gboolean
complete_connection (NMDevice *device,
NMConnection *connection,
const char *specific_object,
const GSList *existing_connections,
NMConnection *const*existing_connections,
GError **error)
{
NMSettingTun *s_tun;

View file

@ -379,7 +379,7 @@ static gboolean
complete_connection (NMDevice *device,
NMConnection *connection,
const char *specific_object,
const GSList *existing_connections,
NMConnection *const*existing_connections,
GError **error)
{
NMSettingVlan *s_vlan;

View file

@ -314,7 +314,7 @@ static gboolean
complete_connection (NMDevice *device,
NMConnection *connection,
const char *specific_object,
const GSList *existing_connections,
NMConnection *const*existing_connections,
GError **error)
{
NMSettingVxlan *s_vxlan;

View file

@ -4781,7 +4781,7 @@ gboolean
nm_device_complete_connection (NMDevice *self,
NMConnection *connection,
const char *specific_object,
const GSList *existing_connections,
NMConnection *const*existing_connections,
GError **error)
{
NMDeviceClass *klass;

View file

@ -312,7 +312,7 @@ typedef struct {
gboolean (* complete_connection) (NMDevice *self,
NMConnection *connection,
const char *specific_object,
const GSList *existing_connections,
NMConnection *const*existing_connections,
GError **error);
NMActStageReturn (* act_stage1_prepare) (NMDevice *self,
@ -520,7 +520,7 @@ gboolean nm_device_can_auto_connect (NMDevice *self,
gboolean nm_device_complete_connection (NMDevice *device,
NMConnection *connection,
const char *specific_object,
const GSList *existing_connection,
NMConnection *const*existing_connections,
GError **error);
gboolean nm_device_check_connection_compatible (NMDevice *device, NMConnection *connection);

View file

@ -104,7 +104,7 @@ static gboolean
complete_connection (NMDevice *device,
NMConnection *connection,
const char *specific_object,
const GSList *existing_connections,
NMConnection *const*existing_connections,
GError **error)
{
NMSettingTeam *s_team;

View file

@ -644,7 +644,7 @@ static gboolean
complete_connection (NMDevice *device,
NMConnection *connection,
const char *specific_object,
const GSList *existing_connections,
NMConnection *const*existing_connections,
GError **error)
{
NMDeviceIwd *self = NM_DEVICE_IWD (device);

View file

@ -113,7 +113,7 @@ static gboolean
complete_connection (NMDevice *device,
NMConnection *connection,
const char *specific_object,
const GSList *existing_connections,
NMConnection *const*existing_connections,
GError **error)
{
NMSettingOlpcMesh *s_mesh;

View file

@ -733,7 +733,7 @@ static gboolean
complete_connection (NMDevice *device,
NMConnection *connection,
const char *specific_object,
const GSList *existing_connections,
NMConnection *const*existing_connections,
GError **error)
{
NMDeviceWifi *self = NM_DEVICE_WIFI (device);

View file

@ -434,7 +434,7 @@ static gboolean
complete_connection (NMDevice *device,
NMConnection *connection,
const char *specific_object,
const GSList *existing_connections,
NMConnection *const*existing_connections,
GError **error)
{
NMDeviceModemPrivate *priv = NM_DEVICE_MODEM_GET_PRIVATE ((NMDeviceModem *) device);

View file

@ -663,7 +663,7 @@ check_connection_compatible (NMModem *_self, NMConnection *connection)
static gboolean
complete_connection (NMModem *_self,
NMConnection *connection,
const GSList *existing_connections,
NMConnection *const*existing_connections,
GError **error)
{
NMModemBroadband *self = NM_MODEM_BROADBAND (_self);

View file

@ -1091,7 +1091,7 @@ nm_modem_check_connection_compatible (NMModem *self, NMConnection *connection)
gboolean
nm_modem_complete_connection (NMModem *self,
NMConnection *connection,
const GSList *existing_connections,
NMConnection *const*existing_connections,
GError **error)
{
NMModemClass *klass;

View file

@ -126,7 +126,7 @@ typedef struct {
gboolean (*complete_connection) (NMModem *modem,
NMConnection *connection,
const GSList *existing_connections,
NMConnection *const*existing_connections,
GError **error);
NMActStageReturn (*act_stage1_prepare) (NMModem *modem,
@ -189,7 +189,7 @@ gboolean nm_modem_check_connection_compatible (NMModem *self, NMConnection *conn
gboolean nm_modem_complete_connection (NMModem *self,
NMConnection *connection,
const GSList *existing_connections,
NMConnection *const*existing_connections,
GError **error);
void nm_modem_get_route_parameters (NMModem *self,

View file

@ -4515,7 +4515,6 @@ impl_manager_add_and_activate_connection (NMDBusObject *obj,
NMManager *self = NM_MANAGER (obj);
NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self);
NMConnection *connection = NULL;
GSList *all_connections = NULL;
NMActiveConnection *active = NULL;
NMAuthSubject *subject = NULL;
GError *error = NULL;
@ -4554,17 +4553,6 @@ impl_manager_add_and_activate_connection (NMDBusObject *obj,
if (!subject)
goto error;
{
NMSettingsConnection *const*connections;
guint i, len;
connections = nm_settings_get_connections (priv->settings, &len);
all_connections = NULL;
for (i = len; i > 0; ) {
i--;
all_connections = g_slist_prepend (all_connections, connections[i]);
}
}
if (vpn) {
/* Try to fill the VPN's connection setting and name at least */
if (!nm_connection_get_setting_vpn (connection)) {
@ -4578,7 +4566,7 @@ impl_manager_add_and_activate_connection (NMDBusObject *obj,
nm_utils_complete_generic (priv->platform,
connection,
NM_SETTING_VPN_SETTING_NAME,
all_connections,
(NMConnection *const*) nm_settings_get_connections (priv->settings, NULL),
NULL,
_("VPN connection"),
NULL,
@ -4588,12 +4576,10 @@ impl_manager_add_and_activate_connection (NMDBusObject *obj,
if (!nm_device_complete_connection (device,
connection,
specific_object_path,
all_connections,
(NMConnection *const*) nm_settings_get_connections (priv->settings, NULL),
&error))
goto error;
}
g_slist_free (all_connections);
all_connections = NULL;
active = _new_active_connection (self,
connection,
@ -4618,7 +4604,6 @@ impl_manager_add_and_activate_connection (NMDBusObject *obj,
error:
nm_audit_log_connection_op (NM_AUDIT_OP_CONN_ADD_ACTIVATE, NULL, FALSE, NULL, subject, error->message);
g_clear_object (&connection);
g_slist_free (all_connections);
g_clear_object (&subject);
g_clear_object (&active);