From 986ca94a3602fdf8c3c849686b0ae8dc87a00bd6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 28 Aug 2018 13:34:40 +0200 Subject: libnm: assert in nm_utils_is_uuid() for valid argument nm_utils_is_uuid() is public API. Commonly we check input arguments for such functions with g_return_*() assertions. --- libnm-core/nm-utils.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index c9162e971e..47ec7a4ea2 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -4333,6 +4333,8 @@ nm_utils_is_uuid (const char *str) const char *p = str; int num_dashes = 0; + g_return_val_if_fail (str, FALSE); + while (*p) { if (*p == '-') num_dashes++; -- cgit v1.2.1 From 80cb515681089fd80851dd3cd7e4763e0d0b1309 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 24 Aug 2018 12:53:02 +0200 Subject: settings/keyfile: always return path from nms_keyfile_writer_connection() Previously, nms_keyfile_writer_connection() would only return @out_path, if it differed from @existing_path. That might make sense, if we could thereby avoid duplicating @existing_path, however, we never did that optimization. Just consistently always return the path, let the caller deal with this. --- src/settings/plugins/keyfile/nms-keyfile-connection.c | 4 +--- src/settings/plugins/keyfile/nms-keyfile-writer.c | 7 ++----- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/src/settings/plugins/keyfile/nms-keyfile-connection.c b/src/settings/plugins/keyfile/nms-keyfile-connection.c index 2dbce4e0c9..64e94b2654 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-connection.c +++ b/src/settings/plugins/keyfile/nms-keyfile-connection.c @@ -75,9 +75,7 @@ commit_changes (NMSettingsConnection *connection, error)) return FALSE; - /* Update the filename if it changed */ - if ( path - && g_strcmp0 (path, nm_settings_connection_get_filename (connection)) != 0) { + if (!nm_streq0 (path, nm_settings_connection_get_filename (connection))) { gs_free char *old_path = g_strdup (nm_settings_connection_get_filename (connection)); nm_settings_connection_set_filename (connection, path); diff --git a/src/settings/plugins/keyfile/nms-keyfile-writer.c b/src/settings/plugins/keyfile/nms-keyfile-writer.c index 1556f15840..194b97d4fd 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-writer.c +++ b/src/settings/plugins/keyfile/nms-keyfile-writer.c @@ -299,11 +299,6 @@ _internal_write_connection (NMConnection *connection, return FALSE; } - if (out_path && g_strcmp0 (existing_path, path)) { - *out_path = path; /* pass path out to caller */ - path = NULL; - } - if (out_reread || out_reread_same) { gs_unref_object NMConnection *reread = NULL; @@ -335,6 +330,8 @@ _internal_write_connection (NMConnection *connection, NM_SET_OUT (out_reread_same, reread_same); } + NM_SET_OUT (out_path, g_steal_pointer (&path)); + return TRUE; } -- cgit v1.2.1 From 122bb485eecedb024a98bec8062eb56ef798ea33 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 24 Aug 2018 13:39:06 +0200 Subject: settings: remove empty NMSettingsPluginInterface.init() implementations --- src/settings/plugins/ibft/nms-ibft-plugin.c | 6 ------ src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c | 6 ------ 2 files changed, 12 deletions(-) diff --git a/src/settings/plugins/ibft/nms-ibft-plugin.c b/src/settings/plugins/ibft/nms-ibft-plugin.c index 9785ba49c0..906889800e 100644 --- a/src/settings/plugins/ibft/nms-ibft-plugin.c +++ b/src/settings/plugins/ibft/nms-ibft-plugin.c @@ -120,11 +120,6 @@ get_connections (NMSettingsPlugin *config) /*****************************************************************************/ -static void -init (NMSettingsPlugin *config) -{ -} - static void nms_ibft_plugin_init (NMSIbftPlugin *self) { @@ -159,7 +154,6 @@ static void settings_plugin_interface_init (NMSettingsPluginInterface *plugin_iface) { plugin_iface->get_connections = get_connections; - plugin_iface->init = init; } /*****************************************************************************/ diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c index 23856b1cd6..f8c25092f8 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c @@ -1001,11 +1001,6 @@ config_changed_cb (NMConfig *config, /*****************************************************************************/ -static void -init (NMSettingsPlugin *config) -{ -} - static void settings_plugin_ifcfg_init (SettingsPluginIfcfg *plugin) { @@ -1080,7 +1075,6 @@ settings_plugin_interface_init (NMSettingsPluginInterface *plugin_iface) plugin_iface->reload_connections = reload_connections; plugin_iface->get_unmanaged_specs = get_unmanaged_specs; plugin_iface->get_unrecognized_specs = get_unrecognized_specs; - plugin_iface->init = init; } /*****************************************************************************/ -- cgit v1.2.1 From 194e7f8df6ec1723be12ca87cf5c2e7c63749ce0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 24 Aug 2018 14:11:58 +0200 Subject: settings: rename NMSettingsPluginInterface.init() to initialize() The virtual function init() naturally leads to calling the wrapper function nm_settings_plugin_init(). However, such ${TYPE}_init() functions are generated by G_DEFINE_TYPE(). Rename to avoid the naming conflict, which will matter next, when the interface will be converted to a regular GObject class. Note that while these are settings plugin, there is no public or stable API which we need to preserve. --- src/settings/nm-settings-plugin.c | 6 +++--- src/settings/nm-settings-plugin.h | 4 ++-- src/settings/nm-settings.c | 2 +- src/settings/plugins/ifupdown/nms-ifupdown-plugin.c | 6 +++--- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/settings/nm-settings-plugin.c b/src/settings/nm-settings-plugin.c index f6dd2cfbf0..212311387b 100644 --- a/src/settings/nm-settings-plugin.c +++ b/src/settings/nm-settings-plugin.c @@ -65,12 +65,12 @@ nm_settings_plugin_default_init (NMSettingsPluginInterface *g_iface) } void -nm_settings_plugin_init (NMSettingsPlugin *config) +nm_settings_plugin_initialize (NMSettingsPlugin *config) { g_return_if_fail (config != NULL); - if (NM_SETTINGS_PLUGIN_GET_INTERFACE (config)->init) - NM_SETTINGS_PLUGIN_GET_INTERFACE (config)->init (config); + if (NM_SETTINGS_PLUGIN_GET_INTERFACE (config)->initialize) + NM_SETTINGS_PLUGIN_GET_INTERFACE (config)->initialize (config); } GSList * diff --git a/src/settings/nm-settings-plugin.h b/src/settings/nm-settings-plugin.h index 6fba25e7ab..933615cf20 100644 --- a/src/settings/nm-settings-plugin.h +++ b/src/settings/nm-settings-plugin.h @@ -44,7 +44,7 @@ typedef struct { GTypeInterface g_iface; /* Called when the plugin is loaded to initialize it */ - void (*init) (NMSettingsPlugin *config); + void (*initialize) (NMSettingsPlugin *config); /* Returns a GSList of NMSettingsConnection objects that represent * connections the plugin knows about. The returned list is freed by the @@ -113,7 +113,7 @@ typedef struct { GType nm_settings_plugin_get_type (void); -void nm_settings_plugin_init (NMSettingsPlugin *config); +void nm_settings_plugin_initialize (NMSettingsPlugin *config); GSList *nm_settings_plugin_get_connections (NMSettingsPlugin *config); diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index 6ddc9a94ab..8c6d67e5ab 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -625,8 +625,8 @@ add_plugin (NMSettings *self, NMSettingsPlugin *plugin) } priv->plugins = g_slist_append (priv->plugins, g_object_ref (plugin)); - nm_settings_plugin_init (plugin); + nm_settings_plugin_initialize (plugin); path = g_object_get_qdata (G_OBJECT (plugin), plugin_module_path_quark ()); diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c index 0cad156457..2138db24e4 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c @@ -306,7 +306,7 @@ _udev_device_unref (gpointer ptr) } static void -init (NMSettingsPlugin *config) +initialize (NMSettingsPlugin *config) { SettingsPluginIfupdown *self = SETTINGS_PLUGIN_IFUPDOWN (config); SettingsPluginIfupdownPrivate *priv = SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE (self); @@ -497,8 +497,8 @@ settings_plugin_ifupdown_class_init (SettingsPluginIfupdownClass *req_class) static void settings_plugin_interface_init (NMSettingsPluginInterface *plugin_iface) { - plugin_iface->init = init; - plugin_iface->get_connections = get_connections; + plugin_iface->initialize = initialize; + plugin_iface->get_connections = get_connections; plugin_iface->get_unmanaged_specs = get_unmanaged_specs; } -- cgit v1.2.1 From 32442b2661305ae2b99177d516aec18450f2b6ed Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 24 Aug 2018 14:44:43 +0200 Subject: settings: drop unused get_plugin() checks Nowadays, keyfile settings plugin is always loaded. Hence, this function never returns %NULL and the checks always evalute the the same. --- src/settings/nm-settings.c | 32 +------------------------------- 1 file changed, 1 insertion(+), 31 deletions(-) diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index 8c6d67e5ab..1cc64e6cc3 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -526,28 +526,6 @@ nm_settings_get_unmanaged_specs (NMSettings *self) return priv->unmanaged_specs; } -static NMSettingsPlugin * -get_plugin (NMSettings *self, gboolean has_add_connection) -{ - NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self); - GSList *iter; - - g_return_val_if_fail (self != NULL, NULL); - - /* Do any of the plugins support the given capability? */ - for (iter = priv->plugins; iter; iter = iter->next) { - NMSettingsPlugin *plugin = NM_SETTINGS_PLUGIN (iter->data); - - if (!has_add_connection) - return plugin; - - if (NM_SETTINGS_PLUGIN_GET_INTERFACE (iter->data)->add_connection != NULL) - return plugin; - } - - return NULL; -} - static gboolean find_spec (GSList *spec_list, const char *spec) { @@ -1297,14 +1275,6 @@ nm_settings_add_connection_dbus (NMSettings *self, goto done; } - /* Do any of the plugins support adding? */ - if (!get_plugin (self, TRUE)) { - error = g_error_new_literal (NM_SETTINGS_ERROR, - NM_SETTINGS_ERROR_NOT_SUPPORTED, - "None of the registered plugins support add."); - goto done; - } - if (!nm_auth_is_subject_in_acl_set_error (connection, subject, NM_SETTINGS_ERROR, @@ -1884,7 +1854,7 @@ get_property (GObject *object, guint prop_id, : NULL); break; case PROP_CAN_MODIFY: - g_value_set_boolean (value, !!get_plugin (self, TRUE)); + g_value_set_boolean (value, TRUE); break; case PROP_CONNECTIONS: if (priv->connections_loaded) { -- cgit v1.2.1 From 657b0714b8e272129800e9fd406f9fcb150d7c1f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 24 Aug 2018 13:37:06 +0200 Subject: settings: make NMSettingsPlugin a regular GObject instance and not an interface NMSettingsPlugin was a glib interface, not a regular GObject instance. Accordingly, settings plugins would implement this interface instead of subclassing a parent type. Refactor the code, and make NMSettingsPlugin a GObject type. Plugins are now required to subclass this type. Glib interfaces are more cumbersome than helpful. At least, unless there is a good reason for using them. Our settings plugins are all internal API and are entirely under our control. It also means, this change is fine, as there are no implementations outside of this source tree. Using interfaces do would allow more flexibility in implementing the settings plugin. For example, the plugin would be able to derive from any other GObject type, like NMKimchiRefrigerator. But why would we even? Let's not add monster classes that implement house appliances beside NMSettingsPluginInterface. The settings plugin should have one purpose only: being a settings plugin. Hence, requiring it to subclass NMSettingsPlugin is more than resonable. We don't need interfaces for this. Now that NMSettingsPlugin is a regular object instance, it may also have state, and potentially could provide common functionality for the plugin implementation -- if that turns out to be useful. Arguably, an interface can have state too, for example by attaching the state somewhere else (like NMConnection does). But let's just say no. On a minor note, this also avoids some tiny overhead that comes with glib interfaces. --- src/settings/nm-settings-plugin.c | 183 +++++++++++++-------- src/settings/nm-settings-plugin.h | 79 +++++---- src/settings/plugins/ibft/nms-ibft-plugin.c | 25 +-- .../plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c | 59 ++++--- .../plugins/ifupdown/nms-ifupdown-plugin.c | 41 ++--- src/settings/plugins/keyfile/nms-keyfile-plugin.c | 42 +++-- 6 files changed, 224 insertions(+), 205 deletions(-) diff --git a/src/settings/nm-settings-plugin.c b/src/settings/nm-settings-plugin.c index 212311387b..e7275631dc 100644 --- a/src/settings/nm-settings-plugin.c +++ b/src/settings/nm-settings-plugin.c @@ -15,117 +15,94 @@ * with this program; if not, write to the Free Software Foundation, Inc., * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. * - * Copyright (C) 2007 - 2011 Red Hat, Inc. + * Copyright (C) 2007 - 2018 Red Hat, Inc. * Copyright (C) 2008 Novell, Inc. */ #include "nm-default.h" #include "nm-settings-plugin.h" + #include "nm-settings-connection.h" -G_DEFINE_INTERFACE (NMSettingsPlugin, nm_settings_plugin, G_TYPE_OBJECT) +/*****************************************************************************/ -static void -nm_settings_plugin_default_init (NMSettingsPluginInterface *g_iface) -{ - GType iface_type = G_TYPE_FROM_INTERFACE (g_iface); - static gboolean initialized = FALSE; - - if (initialized) - return; - - /* Signals */ - g_signal_new (NM_SETTINGS_PLUGIN_CONNECTION_ADDED, - iface_type, - G_SIGNAL_RUN_FIRST, - G_STRUCT_OFFSET (NMSettingsPluginInterface, connection_added), - NULL, NULL, - g_cclosure_marshal_VOID__OBJECT, - G_TYPE_NONE, 1, - NM_TYPE_SETTINGS_CONNECTION); - - g_signal_new (NM_SETTINGS_PLUGIN_UNMANAGED_SPECS_CHANGED, - iface_type, - G_SIGNAL_RUN_FIRST, - G_STRUCT_OFFSET (NMSettingsPluginInterface, unmanaged_specs_changed), - NULL, NULL, - g_cclosure_marshal_VOID__VOID, - G_TYPE_NONE, 0); - - g_signal_new (NM_SETTINGS_PLUGIN_UNRECOGNIZED_SPECS_CHANGED, - iface_type, - G_SIGNAL_RUN_FIRST, - G_STRUCT_OFFSET (NMSettingsPluginInterface, unrecognized_specs_changed), - NULL, NULL, - g_cclosure_marshal_VOID__VOID, - G_TYPE_NONE, 0); - - initialized = TRUE; -} +enum { + CONNECTION_ADDED, + UNMANAGED_SPECS_CHANGED, + UNRECOGNIZED_SPECS_CHANGED, + + LAST_SIGNAL +}; + +static guint signals[LAST_SIGNAL] = { 0 }; + +G_DEFINE_TYPE (NMSettingsPlugin, nm_settings_plugin, G_TYPE_OBJECT) + +/*****************************************************************************/ void -nm_settings_plugin_initialize (NMSettingsPlugin *config) +nm_settings_plugin_initialize (NMSettingsPlugin *self) { - g_return_if_fail (config != NULL); + g_return_if_fail (NM_IS_SETTINGS_PLUGIN (self)); - if (NM_SETTINGS_PLUGIN_GET_INTERFACE (config)->initialize) - NM_SETTINGS_PLUGIN_GET_INTERFACE (config)->initialize (config); + if (NM_SETTINGS_PLUGIN_GET_CLASS (self)->initialize) + NM_SETTINGS_PLUGIN_GET_CLASS (self)->initialize (self); } GSList * -nm_settings_plugin_get_connections (NMSettingsPlugin *config) +nm_settings_plugin_get_connections (NMSettingsPlugin *self) { - g_return_val_if_fail (config != NULL, NULL); + g_return_val_if_fail (NM_IS_SETTINGS_PLUGIN (self), NULL); - if (NM_SETTINGS_PLUGIN_GET_INTERFACE (config)->get_connections) - return NM_SETTINGS_PLUGIN_GET_INTERFACE (config)->get_connections (config); + if (NM_SETTINGS_PLUGIN_GET_CLASS (self)->get_connections) + return NM_SETTINGS_PLUGIN_GET_CLASS (self)->get_connections (self); return NULL; } gboolean -nm_settings_plugin_load_connection (NMSettingsPlugin *config, +nm_settings_plugin_load_connection (NMSettingsPlugin *self, const char *filename) { - g_return_val_if_fail (config != NULL, FALSE); + g_return_val_if_fail (NM_IS_SETTINGS_PLUGIN (self), FALSE); - if (NM_SETTINGS_PLUGIN_GET_INTERFACE (config)->load_connection) - return NM_SETTINGS_PLUGIN_GET_INTERFACE (config)->load_connection (config, filename); + if (NM_SETTINGS_PLUGIN_GET_CLASS (self)->load_connection) + return NM_SETTINGS_PLUGIN_GET_CLASS (self)->load_connection (self, filename); return FALSE; } void -nm_settings_plugin_reload_connections (NMSettingsPlugin *config) +nm_settings_plugin_reload_connections (NMSettingsPlugin *self) { - g_return_if_fail (config != NULL); + g_return_if_fail (NM_IS_SETTINGS_PLUGIN (self)); - if (NM_SETTINGS_PLUGIN_GET_INTERFACE (config)->reload_connections) - NM_SETTINGS_PLUGIN_GET_INTERFACE (config)->reload_connections (config); + if (NM_SETTINGS_PLUGIN_GET_CLASS (self)->reload_connections) + NM_SETTINGS_PLUGIN_GET_CLASS (self)->reload_connections (self); } GSList * -nm_settings_plugin_get_unmanaged_specs (NMSettingsPlugin *config) +nm_settings_plugin_get_unmanaged_specs (NMSettingsPlugin *self) { - g_return_val_if_fail (config != NULL, NULL); + g_return_val_if_fail (NM_IS_SETTINGS_PLUGIN (self), NULL); - if (NM_SETTINGS_PLUGIN_GET_INTERFACE (config)->get_unmanaged_specs) - return NM_SETTINGS_PLUGIN_GET_INTERFACE (config)->get_unmanaged_specs (config); + if (NM_SETTINGS_PLUGIN_GET_CLASS (self)->get_unmanaged_specs) + return NM_SETTINGS_PLUGIN_GET_CLASS (self)->get_unmanaged_specs (self); return NULL; } GSList * -nm_settings_plugin_get_unrecognized_specs (NMSettingsPlugin *config) +nm_settings_plugin_get_unrecognized_specs (NMSettingsPlugin *self) { - g_return_val_if_fail (config != NULL, NULL); + g_return_val_if_fail (NM_IS_SETTINGS_PLUGIN (self), NULL); - if (NM_SETTINGS_PLUGIN_GET_INTERFACE (config)->get_unrecognized_specs) - return NM_SETTINGS_PLUGIN_GET_INTERFACE (config)->get_unrecognized_specs (config); + if (NM_SETTINGS_PLUGIN_GET_CLASS (self)->get_unrecognized_specs) + return NM_SETTINGS_PLUGIN_GET_CLASS (self)->get_unrecognized_specs (self); return NULL; } /** * nm_settings_plugin_add_connection: - * @config: the #NMSettingsPlugin + * @self: the #NMSettingsPlugin * @connection: the source connection to create a plugin-specific * #NMSettingsConnection from * @save_to_disk: %TRUE to save the connection to disk immediately, %FALSE to @@ -140,22 +117,88 @@ nm_settings_plugin_get_unrecognized_specs (NMSettingsPlugin *config) * Returns: the new #NMSettingsConnection or %NULL */ NMSettingsConnection * -nm_settings_plugin_add_connection (NMSettingsPlugin *config, +nm_settings_plugin_add_connection (NMSettingsPlugin *self, NMConnection *connection, gboolean save_to_disk, GError **error) { - NMSettingsPluginInterface *config_interface; + NMSettingsPluginClass *klass; - g_return_val_if_fail (config != NULL, NULL); + g_return_val_if_fail (NM_IS_SETTINGS_PLUGIN (self), NULL); g_return_val_if_fail (NM_IS_CONNECTION (connection), NULL); - config_interface = NM_SETTINGS_PLUGIN_GET_INTERFACE (config); - if (!config_interface->add_connection) { + klass = NM_SETTINGS_PLUGIN_GET_CLASS (self); + if (!klass->add_connection) { g_set_error_literal (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_NOT_SUPPORTED, "Plugin does not support adding connections"); return NULL; } - return config_interface->add_connection (config, connection, save_to_disk, error); + return klass->add_connection (self, connection, save_to_disk, error); +} + +/*****************************************************************************/ + +void +_nm_settings_plugin_emit_signal_connection_added (NMSettingsPlugin *self, + NMSettingsConnection *sett_conn) +{ + nm_assert (NM_IS_SETTINGS_PLUGIN (self)); + nm_assert (NM_IS_SETTINGS_CONNECTION (sett_conn)); + + g_signal_emit (self, signals[CONNECTION_ADDED], 0, sett_conn); +} + +void +_nm_settings_plugin_emit_signal_unmanaged_specs_changed (NMSettingsPlugin *self) +{ + nm_assert (NM_IS_SETTINGS_PLUGIN (self)); + + g_signal_emit (self, signals[UNMANAGED_SPECS_CHANGED], 0); +} + +void +_nm_settings_plugin_emit_signal_unrecognized_specs_changed (NMSettingsPlugin *self) +{ + nm_assert (NM_IS_SETTINGS_PLUGIN (self)); + + g_signal_emit (self, signals[UNRECOGNIZED_SPECS_CHANGED], 0); +} + +/*****************************************************************************/ + +static void +nm_settings_plugin_init (NMSettingsPlugin *self) +{ +} + +static void +nm_settings_plugin_class_init (NMSettingsPluginClass *klass) +{ + GObjectClass *object_class = G_OBJECT_CLASS (klass); + + signals[CONNECTION_ADDED] = + g_signal_new (NM_SETTINGS_PLUGIN_CONNECTION_ADDED, + G_OBJECT_CLASS_TYPE (object_class), + G_SIGNAL_RUN_FIRST, + 0, NULL, NULL, + g_cclosure_marshal_VOID__OBJECT, + G_TYPE_NONE, 1, + NM_TYPE_SETTINGS_CONNECTION); + + signals[UNMANAGED_SPECS_CHANGED] = + g_signal_new (NM_SETTINGS_PLUGIN_UNMANAGED_SPECS_CHANGED, + G_OBJECT_CLASS_TYPE (object_class), + G_SIGNAL_RUN_FIRST, + 0, NULL, NULL, + g_cclosure_marshal_VOID__VOID, + G_TYPE_NONE, 0); + + signals[UNRECOGNIZED_SPECS_CHANGED] = + g_signal_new (NM_SETTINGS_PLUGIN_UNRECOGNIZED_SPECS_CHANGED, + G_OBJECT_CLASS_TYPE (object_class), + G_SIGNAL_RUN_FIRST, + 0, NULL, NULL, + g_cclosure_marshal_VOID__VOID, + G_TYPE_NONE, 0); } diff --git a/src/settings/nm-settings-plugin.h b/src/settings/nm-settings-plugin.h index 933615cf20..cd56d6d15d 100644 --- a/src/settings/nm-settings-plugin.h +++ b/src/settings/nm-settings-plugin.h @@ -15,53 +15,52 @@ * with this program; if not, write to the Free Software Foundation, Inc., * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. * - * Copyright (C) 2007 - 2011 Red Hat, Inc. + * Copyright (C) 2007 - 2018 Red Hat, Inc. * Copyright (C) 2008 Novell, Inc. */ -#ifndef __NETWORKMANAGER_SETTINGS_PLUGIN_H__ -#define __NETWORKMANAGER_SETTINGS_PLUGIN_H__ +#ifndef __NM_SETTINGS_PLUGIN_H__ +#define __NM_SETTINGS_PLUGIN_H__ #include "nm-connection.h" -/* Plugin's factory function that returns a GObject that implements - * NMSettingsPlugin. - */ -GObject * nm_settings_plugin_factory (void); - #define NM_TYPE_SETTINGS_PLUGIN (nm_settings_plugin_get_type ()) #define NM_SETTINGS_PLUGIN(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), NM_TYPE_SETTINGS_PLUGIN, NMSettingsPlugin)) +#define NM_SETTINGS_PLUGIN_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), NM_TYPE_SETTINGS_PLUGIN, NMSettingsPluginClass)) #define NM_IS_SETTINGS_PLUGIN(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), NM_TYPE_SETTINGS_PLUGIN)) -#define NM_SETTINGS_PLUGIN_GET_INTERFACE(obj) (G_TYPE_INSTANCE_GET_INTERFACE ((obj), NM_TYPE_SETTINGS_PLUGIN, NMSettingsPluginInterface)) +#define NM_IS_SETTINGS_PLUGIN_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), NM_TYPE_SETTINGS_PLUGIN)) +#define NM_SETTINGS_PLUGIN_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), NM_TYPE_SETTINGS_PLUGIN, NMSettingsPluginClass)) -#define NM_SETTINGS_PLUGIN_UNMANAGED_SPECS_CHANGED "unmanaged-specs-changed" +#define NM_SETTINGS_PLUGIN_UNMANAGED_SPECS_CHANGED "unmanaged-specs-changed" #define NM_SETTINGS_PLUGIN_UNRECOGNIZED_SPECS_CHANGED "unrecognized-specs-changed" -#define NM_SETTINGS_PLUGIN_CONNECTION_ADDED "connection-added" +#define NM_SETTINGS_PLUGIN_CONNECTION_ADDED "connection-added" -typedef struct _NMSettingsPlugin NMSettingsPlugin; +typedef struct { + GObject parent; +} NMSettingsPlugin; typedef struct { - GTypeInterface g_iface; + GObjectClass parent; /* Called when the plugin is loaded to initialize it */ - void (*initialize) (NMSettingsPlugin *config); + void (*initialize) (NMSettingsPlugin *plugin); /* Returns a GSList of NMSettingsConnection objects that represent * connections the plugin knows about. The returned list is freed by the * system settings service. */ - GSList * (*get_connections) (NMSettingsPlugin *config); + GSList * (*get_connections) (NMSettingsPlugin *plugin); /* Requests that the plugin load/reload a single connection, if it * recognizes the filename. Returns success or failure. */ - gboolean (*load_connection) (NMSettingsPlugin *config, + gboolean (*load_connection) (NMSettingsPlugin *plugin, const char *filename); /* Requests that the plugin reload all connection files from disk, * and emit signals reflecting new, changed, and removed connections. */ - void (*reload_connections) (NMSettingsPlugin *config); + void (*reload_connections) (NMSettingsPlugin *plugin); /* * Return a string list of specifications of devices which NetworkManager @@ -72,7 +71,7 @@ typedef struct { * Each string in the list must be in one of the formats recognized by * nm_device_spec_match_list(). */ - GSList * (*get_unmanaged_specs) (NMSettingsPlugin *config); + GSList * (*get_unmanaged_specs) (NMSettingsPlugin *plugin); /* * Return a string list of specifications of devices for which at least @@ -84,7 +83,7 @@ typedef struct { * Each string in the list must be in one of the formats recognized by * nm_device_spec_match_list(). */ - GSList * (*get_unrecognized_specs) (NMSettingsPlugin *config); + GSList * (*get_unrecognized_specs) (NMSettingsPlugin *plugin); /* * Initialize the plugin-specific connection and return a new @@ -93,40 +92,40 @@ typedef struct { * storage if @save_to_disk is TRUE. The returned object is owned by the * plugin and must be referenced by the owner if necessary. */ - NMSettingsConnection * (*add_connection) (NMSettingsPlugin *config, + NMSettingsConnection * (*add_connection) (NMSettingsPlugin *plugin, NMConnection *connection, gboolean save_to_disk, GError **error); - - /* Signals */ - - /* Emitted when a new connection has been found by the plugin */ - void (*connection_added) (NMSettingsPlugin *config, - NMSettingsConnection *connection); - - /* Emitted when the list of unmanaged device specifications changes */ - void (*unmanaged_specs_changed) (NMSettingsPlugin *config); - - /* Emitted when the list of devices with unrecognized connections changes */ - void (*unrecognized_specs_changed) (NMSettingsPlugin *config); -} NMSettingsPluginInterface; +} NMSettingsPluginClass; GType nm_settings_plugin_get_type (void); +/* Plugin's factory function that returns a #NMSettingsPlugin */ +NMSettingsPlugin *nm_settings_plugin_factory (void); + void nm_settings_plugin_initialize (NMSettingsPlugin *config); -GSList *nm_settings_plugin_get_connections (NMSettingsPlugin *config); +GSList *nm_settings_plugin_get_connections (NMSettingsPlugin *plugin); -gboolean nm_settings_plugin_load_connection (NMSettingsPlugin *config, +gboolean nm_settings_plugin_load_connection (NMSettingsPlugin *plugin, const char *filename); -void nm_settings_plugin_reload_connections (NMSettingsPlugin *config); +void nm_settings_plugin_reload_connections (NMSettingsPlugin *plugin); -GSList *nm_settings_plugin_get_unmanaged_specs (NMSettingsPlugin *config); -GSList *nm_settings_plugin_get_unrecognized_specs (NMSettingsPlugin *config); +GSList *nm_settings_plugin_get_unmanaged_specs (NMSettingsPlugin *plugin); +GSList *nm_settings_plugin_get_unrecognized_specs (NMSettingsPlugin *plugin); -NMSettingsConnection *nm_settings_plugin_add_connection (NMSettingsPlugin *config, +NMSettingsConnection *nm_settings_plugin_add_connection (NMSettingsPlugin *plugin, NMConnection *connection, gboolean save_to_disk, GError **error); -#endif /* __NETWORKMANAGER_SETTINGS_PLUGIN_H__ */ +/* internal API */ + +void _nm_settings_plugin_emit_signal_connection_added (NMSettingsPlugin *plugin, + NMSettingsConnection *sett_conn); + +void _nm_settings_plugin_emit_signal_unmanaged_specs_changed (NMSettingsPlugin *plugin); + +void _nm_settings_plugin_emit_signal_unrecognized_specs_changed (NMSettingsPlugin *plugin); + +#endif /* __NM_SETTINGS_PLUGIN_H__ */ diff --git a/src/settings/plugins/ibft/nms-ibft-plugin.c b/src/settings/plugins/ibft/nms-ibft-plugin.c index 906889800e..69dd3733e9 100644 --- a/src/settings/plugins/ibft/nms-ibft-plugin.c +++ b/src/settings/plugins/ibft/nms-ibft-plugin.c @@ -42,19 +42,15 @@ typedef struct { } NMSIbftPluginPrivate; struct _NMSIbftPlugin { - GObject parent; + NMSettingsPlugin parent; NMSIbftPluginPrivate _priv; }; struct _NMSIbftPluginClass { - GObjectClass parent; + NMSettingsPluginClass parent; }; -static void settings_plugin_interface_init (NMSettingsPluginInterface *plugin_iface); - -G_DEFINE_TYPE_EXTENDED (NMSIbftPlugin, nms_ibft_plugin, G_TYPE_OBJECT, 0, - G_IMPLEMENT_INTERFACE (NM_TYPE_SETTINGS_PLUGIN, - settings_plugin_interface_init)) +G_DEFINE_TYPE (NMSIbftPlugin, nms_ibft_plugin, NM_TYPE_SETTINGS_PLUGIN); #define NMS_IBFT_PLUGIN_GET_PRIVATE(self) _NM_GET_PRIVATE (self, NMSIbftPlugin, NMS_IS_IBFT_PLUGIN) @@ -143,23 +139,20 @@ dispose (GObject *object) } static void -nms_ibft_plugin_class_init (NMSIbftPluginClass *req_class) +nms_ibft_plugin_class_init (NMSIbftPluginClass *klass) { - GObjectClass *object_class = G_OBJECT_CLASS (req_class); + GObjectClass *object_class = G_OBJECT_CLASS (klass); + NMSettingsPluginClass *plugin_class = NM_SETTINGS_PLUGIN_CLASS (klass); object_class->dispose = dispose; -} -static void -settings_plugin_interface_init (NMSettingsPluginInterface *plugin_iface) -{ - plugin_iface->get_connections = get_connections; + plugin_class->get_connections = get_connections; } /*****************************************************************************/ -G_MODULE_EXPORT GObject * +G_MODULE_EXPORT NMSettingsPlugin * nm_settings_plugin_factory (void) { - return G_OBJECT (g_object_ref (nms_ibft_plugin_get ())); + return NM_SETTINGS_PLUGIN (g_object_ref (nms_ibft_plugin_get ())); } diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c index f8c25092f8..6cac8cb6b8 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c @@ -70,19 +70,15 @@ typedef struct { } SettingsPluginIfcfgPrivate; struct _SettingsPluginIfcfg { - GObject parent; + NMSettingsPlugin parent; SettingsPluginIfcfgPrivate _priv; }; struct _SettingsPluginIfcfgClass { - GObjectClass parent; + NMSettingsPluginClass parent; }; -static void settings_plugin_interface_init (NMSettingsPluginInterface *plugin_iface); - -G_DEFINE_TYPE_EXTENDED (SettingsPluginIfcfg, settings_plugin_ifcfg, G_TYPE_OBJECT, 0, - G_IMPLEMENT_INTERFACE (NM_TYPE_SETTINGS_PLUGIN, - settings_plugin_interface_init)) +G_DEFINE_TYPE (SettingsPluginIfcfg, settings_plugin_ifcfg, NM_TYPE_SETTINGS_PLUGIN) #define SETTINGS_PLUGIN_IFCFG_GET_PRIVATE(self) _NM_GET_PRIVATE (self, SettingsPluginIfcfg, SETTINGS_IS_PLUGIN_IFCFG) @@ -164,9 +160,9 @@ remove_connection (SettingsPluginIfcfg *self, NMIfcfgConnection *connection) /* Emit changes _after_ removing the connection */ if (unmanaged) - g_signal_emit_by_name (self, NM_SETTINGS_PLUGIN_UNMANAGED_SPECS_CHANGED); + _nm_settings_plugin_emit_signal_unmanaged_specs_changed (NM_SETTINGS_PLUGIN (self)); if (unrecognized) - g_signal_emit_by_name (self, NM_SETTINGS_PLUGIN_UNRECOGNIZED_SPECS_CHANGED); + _nm_settings_plugin_emit_signal_unrecognized_specs_changed (NM_SETTINGS_PLUGIN (self)); } static NMIfcfgConnection * @@ -349,18 +345,20 @@ update_connection (SettingsPluginIfcfg *self, if (old_unmanaged /* && !new_unmanaged */) { _LOGI ("Managing connection "NM_IFCFG_CONNECTION_LOG_FMT" and its device because NM_CONTROLLED was true.", NM_IFCFG_CONNECTION_LOG_ARG (connection_new)); - g_signal_emit_by_name (self, NM_SETTINGS_PLUGIN_CONNECTION_ADDED, connection_by_uuid); + _nm_settings_plugin_emit_signal_connection_added (NM_SETTINGS_PLUGIN (self), + NM_SETTINGS_CONNECTION (connection_by_uuid)); } else if (old_unrecognized /* && !new_unrecognized */) { _LOGI ("Managing connection "NM_IFCFG_CONNECTION_LOG_FMT" because it is now a recognized type.", NM_IFCFG_CONNECTION_LOG_ARG (connection_new)); - g_signal_emit_by_name (self, NM_SETTINGS_PLUGIN_CONNECTION_ADDED, connection_by_uuid); + _nm_settings_plugin_emit_signal_connection_added (NM_SETTINGS_PLUGIN (self), + NM_SETTINGS_CONNECTION (connection_by_uuid)); } } if (unmanaged_changed) - g_signal_emit_by_name (self, NM_SETTINGS_PLUGIN_UNMANAGED_SPECS_CHANGED); + _nm_settings_plugin_emit_signal_unmanaged_specs_changed (NM_SETTINGS_PLUGIN (self)); if (unrecognized_changed) - g_signal_emit_by_name (self, NM_SETTINGS_PLUGIN_UNRECOGNIZED_SPECS_CHANGED); + _nm_settings_plugin_emit_signal_unrecognized_specs_changed (NM_SETTINGS_PLUGIN (self)); } nm_settings_connection_set_filename (NM_SETTINGS_CONNECTION (connection_by_uuid), full_path); g_object_unref (connection_new); @@ -398,11 +396,13 @@ update_connection (SettingsPluginIfcfg *self, /* Only raise the signal if we were called without source, i.e. if we read the connection from file. * Otherwise, we were called by add_connection() which does not expect the signal. */ if (nm_ifcfg_connection_get_unmanaged_spec (connection_new)) - g_signal_emit_by_name (self, NM_SETTINGS_PLUGIN_UNMANAGED_SPECS_CHANGED); + _nm_settings_plugin_emit_signal_unmanaged_specs_changed (NM_SETTINGS_PLUGIN (self)); else if (nm_ifcfg_connection_get_unrecognized_spec (connection_new)) - g_signal_emit_by_name (self, NM_SETTINGS_PLUGIN_UNRECOGNIZED_SPECS_CHANGED); - else - g_signal_emit_by_name (self, NM_SETTINGS_PLUGIN_CONNECTION_ADDED, connection_new); + _nm_settings_plugin_emit_signal_unrecognized_specs_changed (NM_SETTINGS_PLUGIN (self)); + else { + _nm_settings_plugin_emit_signal_connection_added (NM_SETTINGS_PLUGIN (self), + NM_SETTINGS_CONNECTION (connection_new)); + } } return connection_new; } @@ -1058,29 +1058,26 @@ dispose (GObject *object) } static void -settings_plugin_ifcfg_class_init (SettingsPluginIfcfgClass *req_class) +settings_plugin_ifcfg_class_init (SettingsPluginIfcfgClass *klass) { - GObjectClass *object_class = G_OBJECT_CLASS (req_class); + GObjectClass *object_class = G_OBJECT_CLASS (klass); + NMSettingsPluginClass *plugin_class = NM_SETTINGS_PLUGIN_CLASS (klass); object_class->constructed = constructed; object_class->dispose = dispose; -} -static void -settings_plugin_interface_init (NMSettingsPluginInterface *plugin_iface) -{ - plugin_iface->get_connections = get_connections; - plugin_iface->add_connection = add_connection; - plugin_iface->load_connection = load_connection; - plugin_iface->reload_connections = reload_connections; - plugin_iface->get_unmanaged_specs = get_unmanaged_specs; - plugin_iface->get_unrecognized_specs = get_unrecognized_specs; + plugin_class->get_connections = get_connections; + plugin_class->add_connection = add_connection; + plugin_class->load_connection = load_connection; + plugin_class->reload_connections = reload_connections; + plugin_class->get_unmanaged_specs = get_unmanaged_specs; + plugin_class->get_unrecognized_specs = get_unrecognized_specs; } /*****************************************************************************/ -G_MODULE_EXPORT GObject * +G_MODULE_EXPORT NMSettingsPlugin * nm_settings_plugin_factory (void) { - return G_OBJECT (g_object_ref (settings_plugin_ifcfg_get ())); + return NM_SETTINGS_PLUGIN (g_object_ref (settings_plugin_ifcfg_get ())); } diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c index 2138db24e4..4970ccbc4e 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c @@ -76,19 +76,15 @@ typedef struct { } SettingsPluginIfupdownPrivate; struct _SettingsPluginIfupdown { - GObject parent; + NMSettingsPlugin parent; SettingsPluginIfupdownPrivate _priv; }; struct _SettingsPluginIfupdownClass { - GObjectClass parent; + NMSettingsPluginClass parent; }; -static void settings_plugin_interface_init (NMSettingsPluginInterface *plugin_iface); - -G_DEFINE_TYPE_EXTENDED (SettingsPluginIfupdown, settings_plugin_ifupdown, G_TYPE_OBJECT, 0, - G_IMPLEMENT_INTERFACE (NM_TYPE_SETTINGS_PLUGIN, - settings_plugin_interface_init)) +G_DEFINE_TYPE (SettingsPluginIfupdown, settings_plugin_ifupdown, NM_TYPE_SETTINGS_PLUGIN) #define SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE(self) _NM_GET_PRIVATE (self, SettingsPluginIfupdown, SETTINGS_IS_PLUGIN_IFUPDOWN) @@ -174,7 +170,7 @@ udev_device_added (SettingsPluginIfupdown *self, struct udev_device *device) bind_device_to_connection (self, device, exported); if (ALWAYS_UNMANAGE || priv->unmanage_well_known) - g_signal_emit_by_name (G_OBJECT (self), NM_SETTINGS_PLUGIN_UNMANAGED_SPECS_CHANGED); + _nm_settings_plugin_emit_signal_unmanaged_specs_changed (NM_SETTINGS_PLUGIN (self)); } static void @@ -194,7 +190,7 @@ udev_device_removed (SettingsPluginIfupdown *self, struct udev_device *device) return; if (ALWAYS_UNMANAGE || priv->unmanage_well_known) - g_signal_emit_by_name (G_OBJECT (self), NM_SETTINGS_PLUGIN_UNMANAGED_SPECS_CHANGED); + _nm_settings_plugin_emit_signal_unmanaged_specs_changed (NM_SETTINGS_PLUGIN (self)); } static void @@ -214,7 +210,7 @@ udev_device_changed (SettingsPluginIfupdown *self, struct udev_device *device) return; if (ALWAYS_UNMANAGE || priv->unmanage_well_known) - g_signal_emit_by_name (G_OBJECT (self), NM_SETTINGS_PLUGIN_UNMANAGED_SPECS_CHANGED); + _nm_settings_plugin_emit_signal_unmanaged_specs_changed (NM_SETTINGS_PLUGIN (self)); } static void @@ -455,9 +451,8 @@ initialize (NMSettingsPlugin *config) GList *cl_iter; for (cl_iter = con_list; cl_iter; cl_iter = g_list_next (cl_iter)) { - g_signal_emit_by_name (self, - NM_SETTINGS_PLUGIN_CONNECTION_ADDED, - NM_SETTINGS_CONNECTION (cl_iter->data)); + _nm_settings_plugin_emit_signal_connection_added (NM_SETTINGS_PLUGIN (self), + NM_SETTINGS_CONNECTION (cl_iter->data)); } g_list_free (con_list); } @@ -487,26 +482,22 @@ dispose (GObject *object) } static void -settings_plugin_ifupdown_class_init (SettingsPluginIfupdownClass *req_class) +settings_plugin_ifupdown_class_init (SettingsPluginIfupdownClass *klass) { - GObjectClass *object_class = G_OBJECT_CLASS (req_class); + GObjectClass *object_class = G_OBJECT_CLASS (klass); + NMSettingsPluginClass *plugin_class = NM_SETTINGS_PLUGIN_CLASS (klass); object_class->dispose = dispose; -} -static void -settings_plugin_interface_init (NMSettingsPluginInterface *plugin_iface) -{ - plugin_iface->initialize = initialize; - plugin_iface->get_connections = get_connections; - plugin_iface->get_unmanaged_specs = get_unmanaged_specs; + plugin_class->initialize = initialize; + plugin_class->get_connections = get_connections; + plugin_class->get_unmanaged_specs = get_unmanaged_specs; } /*****************************************************************************/ -G_MODULE_EXPORT GObject * +G_MODULE_EXPORT NMSettingsPlugin * nm_settings_plugin_factory (void) { - return G_OBJECT (g_object_ref (settings_plugin_ifupdown_get ())); + return NM_SETTINGS_PLUGIN (g_object_ref (settings_plugin_ifupdown_get ())); } - diff --git a/src/settings/plugins/keyfile/nms-keyfile-plugin.c b/src/settings/plugins/keyfile/nms-keyfile-plugin.c index d97edd1ad4..89b894674e 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-plugin.c +++ b/src/settings/plugins/keyfile/nms-keyfile-plugin.c @@ -56,19 +56,15 @@ typedef struct { } NMSKeyfilePluginPrivate; struct _NMSKeyfilePlugin { - GObject parent; + NMSettingsPlugin parent; NMSKeyfilePluginPrivate _priv; }; struct _NMSKeyfilePluginClass { - GObjectClass parent; + NMSettingsPluginClass parent; }; -static void settings_plugin_interface_init (NMSettingsPluginInterface *plugin_iface); - -G_DEFINE_TYPE_EXTENDED (NMSKeyfilePlugin, nms_keyfile_plugin, G_TYPE_OBJECT, 0, - G_IMPLEMENT_INTERFACE (NM_TYPE_SETTINGS_PLUGIN, - settings_plugin_interface_init)) +G_DEFINE_TYPE (NMSKeyfilePlugin, nms_keyfile_plugin, NM_TYPE_SETTINGS_PLUGIN) #define NMS_KEYFILE_PLUGIN_GET_PRIVATE(self) _NM_GET_PRIVATE (self, NMSKeyfilePlugin, NMS_IS_KEYFILE_PLUGIN) @@ -287,8 +283,10 @@ update_connection (NMSKeyfilePlugin *self, if (!source) { /* Only raise the signal if we were called without source, i.e. if we read the connection from file. * Otherwise, we were called by add_connection() which does not expect the signal. */ - g_signal_emit_by_name (self, NM_SETTINGS_PLUGIN_CONNECTION_ADDED, connection_new); + _nm_settings_plugin_emit_signal_connection_added (NM_SETTINGS_PLUGIN (self), + NM_SETTINGS_CONNECTION (connection_new)); } + return connection_new; } } @@ -341,13 +339,14 @@ config_changed_cb (NMConfig *config, NMConfigData *old_data, NMSKeyfilePlugin *self) { - gs_free char *old_value = NULL, *new_value = NULL; + gs_free char *old_value = NULL; + gs_free char *new_value = NULL; old_value = nm_config_data_get_value (old_data, NM_CONFIG_KEYFILE_GROUP_KEYFILE, NM_CONFIG_KEYFILE_KEY_KEYFILE_UNMANAGED_DEVICES, NM_CONFIG_GET_VALUE_TYPE_SPEC); new_value = nm_config_data_get_value (config_data, NM_CONFIG_KEYFILE_GROUP_KEYFILE, NM_CONFIG_KEYFILE_KEY_KEYFILE_UNMANAGED_DEVICES, NM_CONFIG_GET_VALUE_TYPE_SPEC); - if (g_strcmp0 (old_value, new_value) != 0) - g_signal_emit_by_name (self, NM_SETTINGS_PLUGIN_UNMANAGED_SPECS_CHANGED); + if (!nm_streq0 (old_value, new_value)) + _nm_settings_plugin_emit_signal_unmanaged_specs_changed (NM_SETTINGS_PLUGIN (self)); } static void @@ -616,20 +615,17 @@ dispose (GObject *object) } static void -nms_keyfile_plugin_class_init (NMSKeyfilePluginClass *req_class) +nms_keyfile_plugin_class_init (NMSKeyfilePluginClass *klass) { - GObjectClass *object_class = G_OBJECT_CLASS (req_class); + GObjectClass *object_class = G_OBJECT_CLASS (klass); + NMSettingsPluginClass *plugin_class = NM_SETTINGS_PLUGIN_CLASS (klass); object_class->constructed = constructed; - object_class->dispose = dispose; -} + object_class->dispose = dispose; -static void -settings_plugin_interface_init (NMSettingsPluginInterface *plugin_iface) -{ - plugin_iface->get_connections = get_connections; - plugin_iface->load_connection = load_connection; - plugin_iface->reload_connections = reload_connections; - plugin_iface->add_connection = add_connection; - plugin_iface->get_unmanaged_specs = get_unmanaged_specs; + plugin_class->get_connections = get_connections; + plugin_class->load_connection = load_connection; + plugin_class->reload_connections = reload_connections; + plugin_class->add_connection = add_connection; + plugin_class->get_unmanaged_specs = get_unmanaged_specs; } -- cgit v1.2.1 From dd5244af3efa9af13357a357768aae0635a80a34 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 28 Aug 2018 13:35:47 +0200 Subject: settings: disconnect signals from plugins when destroying NMSettings Currently we anyway leak everything on shutdown, so this doesn't matter. But to be correct, we must disconnect signal handlers. --- src/settings/nm-settings.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index 1cc64e6cc3..becb4ed1ec 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -1918,6 +1918,7 @@ finalize (GObject *object) { NMSettings *self = NM_SETTINGS (object); NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self); + GSList *iter; _clear_connections_cached_list (priv); @@ -1926,7 +1927,12 @@ finalize (GObject *object) g_slist_free_full (priv->unmanaged_specs, g_free); g_slist_free_full (priv->unrecognized_specs, g_free); - g_slist_free_full (priv->plugins, g_object_unref); + while ((iter = priv->plugins)) { + gs_unref_object NMSettingsPlugin *plugin = iter->data; + + priv->plugins = g_slist_delete_link (priv->plugins, iter); + g_signal_handlers_disconnect_by_data (plugin, self); + } g_clear_object (&priv->agent_mgr); -- cgit v1.2.1 From 0ea810fa96de56381d1be15b48f8b18dabb29274 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 5 Sep 2018 09:19:39 +0200 Subject: settings: cleanup loading settings plugins Drop the unnecessary @list argument and various cleanups. --- src/settings/nm-settings-plugin.h | 2 + src/settings/nm-settings.c | 110 +++++++++++++++----------------------- 2 files changed, 46 insertions(+), 66 deletions(-) diff --git a/src/settings/nm-settings-plugin.h b/src/settings/nm-settings-plugin.h index cd56d6d15d..fdd48f2bc9 100644 --- a/src/settings/nm-settings-plugin.h +++ b/src/settings/nm-settings-plugin.h @@ -100,6 +100,8 @@ typedef struct { GType nm_settings_plugin_get_type (void); +typedef NMSettingsPlugin *(*NMSettingsPluginFactoryFunc) (void); + /* Plugin's factory function that returns a #NMSettingsPlugin */ NMSettingsPlugin *nm_settings_plugin_factory (void); diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index becb4ed1ec..2253d56a84 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -586,67 +586,49 @@ unrecognized_specs_changed (NMSettingsPlugin *config, nm_settings_plugin_get_unrecognized_specs); } -static gboolean -add_plugin (NMSettings *self, NMSettingsPlugin *plugin) +static void +add_plugin (NMSettings *self, NMSettingsPlugin *plugin, const char *path) { NMSettingsPrivate *priv; - const char *path; - g_return_val_if_fail (NM_IS_SETTINGS (self), FALSE); - g_return_val_if_fail (NM_IS_SETTINGS_PLUGIN (plugin), FALSE); + nm_assert (NM_IS_SETTINGS (self)); + nm_assert (NM_IS_SETTINGS_PLUGIN (plugin)); priv = NM_SETTINGS_GET_PRIVATE (self); - if (g_slist_find (priv->plugins, plugin)) { - /* don't add duplicates. */ - return FALSE; - } + nm_assert (!g_slist_find (priv->plugins, plugin)); priv->plugins = g_slist_append (priv->plugins, g_object_ref (plugin)); nm_settings_plugin_initialize (plugin); - path = g_object_get_qdata (G_OBJECT (plugin), plugin_module_path_quark ()); - - _LOGI ("Loaded settings plugin: %s (%s)", G_OBJECT_TYPE_NAME (plugin), path ?: "internal"); - - return TRUE; + _LOGI ("Loaded settings plugin: %s (%s%s%s)", + G_OBJECT_TYPE_NAME (plugin), + NM_PRINT_FMT_QUOTED (path, "\"", path, "\"", "internal")); } static gboolean -plugin_loaded (GSList *list, const char *path) -{ - GSList *iter; - - g_return_val_if_fail (path != NULL, TRUE); - - for (iter = list; iter; iter = g_slist_next (iter)) { - const char *list_path = g_object_get_qdata (G_OBJECT (iter->data), - plugin_module_path_quark ()); - - if (g_strcmp0 (path, list_path) == 0) - return TRUE; - } - - return FALSE; -} - -static gboolean -load_plugin (NMSettings *self, GSList **list, const char *pname, GError **error) +add_plugin_load_file (NMSettings *self, const char *pname, GError **error) { + NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self); gs_free char *full_name = NULL; gs_free char *path = NULL; - gs_unref_object GObject *obj = NULL; - GModule *plugin; - GObject * (*factory_func) (void); + gs_unref_object NMSettingsPlugin *plugin = NULL; + GModule *module; + NMSettingsPluginFactoryFunc factory_func; + GSList *iter; struct stat st; int errsv; full_name = g_strdup_printf ("nm-settings-plugin-%s", pname); path = g_module_build_path (NMPLUGINDIR, full_name); - if (plugin_loaded (*list, path)) - return TRUE; + for (iter = priv->plugins; iter; iter = iter->next) { + if (nm_streq0 (path, + g_object_get_qdata (iter->data, + plugin_module_path_quark ()))) + return TRUE; + } if (stat (path, &st) != 0) { errsv = errno; @@ -666,8 +648,8 @@ load_plugin (NMSettings *self, GSList **list, const char *pname, GError **error) return TRUE; } - plugin = g_module_open (path, G_MODULE_BIND_LOCAL); - if (!plugin) { + module = g_module_open (path, G_MODULE_BIND_LOCAL); + if (!module) { _LOGW ("could not load plugin '%s' from file '%s': %s", pname, path, g_module_error ()); return TRUE; @@ -675,48 +657,46 @@ load_plugin (NMSettings *self, GSList **list, const char *pname, GError **error) /* errors after this point are fatal, because we loaded the shared library already. */ - if (!g_module_symbol (plugin, "nm_settings_plugin_factory", (gpointer) (&factory_func))) { + if (!g_module_symbol (module, "nm_settings_plugin_factory", (gpointer) (&factory_func))) { g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED, "Could not find plugin '%s' factory function.", pname); - g_module_close (plugin); + g_module_close (module); return FALSE; } /* after accessing the plugin we cannot unload it anymore, because the glib * types cannot be properly unregistered. */ - g_module_make_resident (plugin); + g_module_make_resident (module); - obj = (*factory_func) (); - if (!obj || !NM_IS_SETTINGS_PLUGIN (obj)) { + plugin = (*factory_func) (); + if (!NM_IS_SETTINGS_PLUGIN (plugin)) { g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED, - "Plugin '%s' returned invalid system config object.", + "plugin '%s' returned invalid settings plugin", pname); return FALSE; } - g_object_set_qdata_full (obj, plugin_module_path_quark (), path, g_free); - path = NULL; - if (add_plugin (self, NM_SETTINGS_PLUGIN (obj))) - *list = g_slist_append (*list, g_steal_pointer (&obj)); - + add_plugin (self, NM_SETTINGS_PLUGIN (plugin), path); + g_object_set_qdata_full (G_OBJECT (plugin), + plugin_module_path_quark (), + g_steal_pointer (&path), + g_free); return TRUE; } static void -add_keyfile_plugin (NMSettings *self) +add_plugin_keyfile (NMSettings *self) { gs_unref_object NMSKeyfilePlugin *keyfile_plugin = NULL; keyfile_plugin = nms_keyfile_plugin_new (); - if (!add_plugin (self, NM_SETTINGS_PLUGIN (keyfile_plugin))) - g_return_if_reached (); + add_plugin (self, NM_SETTINGS_PLUGIN (keyfile_plugin), NULL); } static gboolean load_plugins (NMSettings *self, const char **plugins, GError **error) { - GSList *list = NULL; const char **iter; gboolean keyfile_added = FALSE; gboolean success = TRUE; @@ -744,15 +724,15 @@ load_plugins (NMSettings *self, const char **plugins, GError **error) continue; } - if (!strcmp (pname, "no-ibft")) + if (nm_streq (pname, "no-ibft")) continue; - if (has_no_ibft && !strcmp (pname, "ibft")) + if (has_no_ibft && nm_streq (pname, "ibft")) continue; /* keyfile plugin is built-in now */ - if (strcmp (pname, "keyfile") == 0) { + if (nm_streq (pname, "keyfile")) { if (!keyfile_added) { - add_keyfile_plugin (self); + add_plugin_keyfile (self); keyfile_added = TRUE; } continue; @@ -766,27 +746,25 @@ load_plugins (NMSettings *self, const char **plugins, GError **error) continue; } - success = load_plugin (self, &list, pname, error); + success = add_plugin_load_file (self, pname, error); if (!success) break; - if (add_ibft && !strcmp (pname, "ifcfg-rh")) { + if (add_ibft && nm_streq (pname, "ifcfg-rh")) { /* The plugin ibft is not explicitly mentioned but we just enabled "ifcfg-rh". * Enable "ibft" by default after "ifcfg-rh". */ pname = "ibft"; add_ibft = FALSE; - success = load_plugin (self, &list, "ibft", error); + success = add_plugin_load_file (self, "ibft", error); if (!success) break; } } /* If keyfile plugin was not among configured plugins, add it as the last one */ - if (!keyfile_added) - add_keyfile_plugin (self); - - g_slist_free_full (list, g_object_unref); + if (!keyfile_added && success) + add_plugin_keyfile (self); return success; } -- cgit v1.2.1 From 42c2055a313a613b8b8122acbf72a7230a9bb045 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 25 Aug 2018 10:00:48 +0200 Subject: settings/ifupdown: cleanup lifetime and memory handling of dictionaries in plugin - initialize the hash tables in the plugins constructor, not during initialize(). - let all dictionaries own a copy/reference of the keys and values, and properly free them when the values are removed. In general, avoid leaks by properly managing lifetimes. - in @eni_ifaces, don't add a pointless dummy value "known". It has overhead for no benefit. --- .../plugins/ifupdown/nms-ifupdown-plugin.c | 28 ++++++++++------------ 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c index 4970ccbc4e..8e57ff6453 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c @@ -158,7 +158,8 @@ udev_device_added (SettingsPluginIfupdown *self, struct udev_device *device) * we want to either unmanage the device or lock it */ exported = g_hash_table_lookup (priv->connections, iface); - if (!exported && !g_hash_table_lookup (priv->eni_ifaces, iface)) { + if ( !exported + && !g_hash_table_contains (priv->eni_ifaces, iface)) { nm_log_info (LOGD_SETTINGS, "device added (path: %s, iface: %s): no ifupdown configuration found.", path, iface); return; @@ -316,15 +317,6 @@ initialize (NMSettingsPlugin *config) auto_ifaces = g_hash_table_new (nm_str_hash, g_str_equal); - if(!priv->connections) - priv->connections = g_hash_table_new (nm_str_hash, g_str_equal); - - if(!priv->kernel_ifaces) - priv->kernel_ifaces = g_hash_table_new_full (nm_str_hash, g_str_equal, g_free, _udev_device_unref); - - if(!priv->eni_ifaces) - priv->eni_ifaces = g_hash_table_new (nm_str_hash, g_str_equal); - nm_log_info (LOGD_SETTINGS, "init!"); priv->udev_client = nm_udev_client_new ((const char *[]) { "net", NULL }, @@ -371,7 +363,7 @@ initialize (NMSettingsPlugin *config) } if (state == 0 && strlen (token) > 0) { nm_log_info (LOGD_SETTINGS, "adding bridge port %s to eni_ifaces", token); - g_hash_table_insert (priv->eni_ifaces, g_strdup (token), "known"); + g_hash_table_add (priv->eni_ifaces, g_strdup (token)); } } g_strfreev (port_ifaces); @@ -396,12 +388,12 @@ initialize (NMSettingsPlugin *config) exported = nm_ifupdown_connection_new (block); if (exported) { nm_log_info (LOGD_SETTINGS, "adding %s to connections", block->name); - g_hash_table_insert (priv->connections, block->name, exported); + g_hash_table_insert (priv->connections, g_strdup (block->name), exported); } nm_log_info (LOGD_SETTINGS, "adding iface %s to eni_ifaces", block->name); - g_hash_table_insert (priv->eni_ifaces, block->name, "known"); + g_hash_table_add (priv->eni_ifaces, g_strdup (block->name)); } else if (!strcmp ("mapping", block->type)) { - g_hash_table_insert (priv->eni_ifaces, block->name, "known"); + g_hash_table_add (priv->eni_ifaces, g_strdup (block->name)); nm_log_info (LOGD_SETTINGS, "adding mapping %s to eni_ifaces", block->name); } next: @@ -463,8 +455,13 @@ initialize (NMSettingsPlugin *config) /*****************************************************************************/ static void -settings_plugin_ifupdown_init (SettingsPluginIfupdown *plugin) +settings_plugin_ifupdown_init (SettingsPluginIfupdown *self) { + SettingsPluginIfupdownPrivate *priv = SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE (self); + + priv->connections = g_hash_table_new_full (nm_str_hash, g_str_equal, g_free, g_object_unref); + priv->kernel_ifaces = g_hash_table_new_full (nm_str_hash, g_str_equal, g_free, _udev_device_unref); + priv->eni_ifaces = g_hash_table_new_full (nm_str_hash, g_str_equal, g_free, NULL); } static void @@ -473,6 +470,7 @@ dispose (GObject *object) SettingsPluginIfupdown *plugin = SETTINGS_PLUGIN_IFUPDOWN (object); SettingsPluginIfupdownPrivate *priv = SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE (plugin); + g_clear_pointer (&priv->connections, g_hash_table_destroy); g_clear_pointer (&priv->kernel_ifaces, g_hash_table_destroy); g_clear_pointer (&priv->eni_ifaces, g_hash_table_destroy); -- cgit v1.2.1 From 553c3368aba05da0d0bcc4174a1cc4bd3bc72782 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 25 Aug 2018 10:08:16 +0200 Subject: settings/ifupdown: minor cleanup of auto-ifaces in plugin's initialize() - use gs_unref_hashtable for managing lifetime - only allocate the hashtable if necessary, and use g_hash_table_add() which is optimized by HashTable. - actually copy the block->name that is used as key. While not necessary at the moment, it is very ugly how ifparser_getfirst() returns static data. Optimally, this would be fixed and we create and destroy the parser results. Hence, ensure the lifetime of the key. --- .../plugins/ifupdown/nms-ifupdown-plugin.c | 27 +++++++++++----------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c index 8e57ff6453..acb62932ea 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c @@ -307,7 +307,7 @@ initialize (NMSettingsPlugin *config) { SettingsPluginIfupdown *self = SETTINGS_PLUGIN_IFUPDOWN (config); SettingsPluginIfupdownPrivate *priv = SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE (self); - GHashTable *auto_ifaces; + gs_unref_hashtable GHashTable *auto_ifaces = NULL; if_block *block = NULL; struct udev_enumerate *enumerate; struct udev_list_entry *keys; @@ -315,8 +315,6 @@ initialize (NMSettingsPlugin *config) const char *block_name; NMIfupdownConnection *connection; - auto_ifaces = g_hash_table_new (nm_str_hash, g_str_equal); - nm_log_info (LOGD_SETTINGS, "init!"); priv->udev_client = nm_udev_client_new ((const char *[]) { "net", NULL }, @@ -326,9 +324,11 @@ initialize (NMSettingsPlugin *config) ifparser_init (ENI_INTERFACES_FILE, 0); block = ifparser_getfirst (); while (block) { - if(!strcmp ("auto", block->type) || !strcmp ("allow-hotplug", block->type)) - g_hash_table_insert (auto_ifaces, block->name, GUINT_TO_POINTER (1)); - else if (!strcmp ("iface", block->type)) { + if(!strcmp ("auto", block->type) || !strcmp ("allow-hotplug", block->type)) { + if (!auto_ifaces) + auto_ifaces = g_hash_table_new_full (nm_str_hash, g_str_equal, g_free, NULL); + g_hash_table_add (auto_ifaces, g_strdup (block->name)); + } else if (!strcmp ("iface", block->type)) { NMIfupdownConnection *exported; /* Bridge configuration */ @@ -405,14 +405,15 @@ initialize (NMSettingsPlugin *config) while (g_hash_table_iter_next (&con_iter, (gpointer) &block_name, (gpointer) &connection)) { NMSettingConnection *setting; - if (g_hash_table_lookup (auto_ifaces, block_name)) { - /* FIXME(copy-on-write-connection): avoid modifying NMConnection instances and share them via copy-on-write. */ - setting = nm_connection_get_setting_connection (nm_settings_connection_get_connection (NM_SETTINGS_CONNECTION (connection))); - g_object_set (setting, NM_SETTING_CONNECTION_AUTOCONNECT, TRUE, NULL); - nm_log_info (LOGD_SETTINGS, "autoconnect"); - } + if ( !auto_ifaces + || !g_hash_table_contains (auto_ifaces, block_name)) + continue; + + /* FIXME(copy-on-write-connection): avoid modifying NMConnection instances and share them via copy-on-write. */ + setting = nm_connection_get_setting_connection (nm_settings_connection_get_connection (NM_SETTINGS_CONNECTION (connection))); + g_object_set (setting, NM_SETTING_CONNECTION_AUTOCONNECT, TRUE, NULL); + nm_log_info (LOGD_SETTINGS, "autoconnect"); } - g_hash_table_destroy (auto_ifaces); /* Check the config file to find out whether to manage interfaces */ priv->unmanage_well_known = !nm_config_data_get_value_boolean (NM_CONFIG_GET_DATA_ORIG, -- cgit v1.2.1 From f0938948bc506f2bddda2d574b0890cb4b67b4c4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 25 Aug 2018 10:17:15 +0200 Subject: settings/ifupdown: replace strcmp() usage with nm_streq()/NM_IN_STRSET() in plugin --- .../plugins/ifupdown/nms-ifupdown-plugin.c | 28 ++++++++++------------ 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c index acb62932ea..564cb83aa9 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c @@ -231,11 +231,11 @@ handle_uevent (NMUdevClient *client, subsys = udev_device_get_subsystem (device); g_return_if_fail (nm_streq0 (subsys, "net")); - if (!strcmp (action, "add")) + if (nm_streq (action, "add")) udev_device_added (self, device); - else if (!strcmp (action, "remove")) + else if (nm_streq (action, "remove")) udev_device_removed (self, device); - else if (!strcmp (action, "change")) + else if (nm_streq (action, "change")) udev_device_changed (self, device); } @@ -250,7 +250,7 @@ get_connections (NMSettingsPlugin *config) nm_log_info (LOGD_SETTINGS, "(%d) ... get_connections.", GPOINTER_TO_UINT(config)); - if(priv->unmanage_well_known) { + if (priv->unmanage_well_known) { nm_log_info (LOGD_SETTINGS, "(%d) ... get_connections (managed=false): return empty list.", GPOINTER_TO_UINT(config)); return NULL; } @@ -324,15 +324,15 @@ initialize (NMSettingsPlugin *config) ifparser_init (ENI_INTERFACES_FILE, 0); block = ifparser_getfirst (); while (block) { - if(!strcmp ("auto", block->type) || !strcmp ("allow-hotplug", block->type)) { + if (NM_IN_STRSET (block->type, "auto", "allow-hotplug")) { if (!auto_ifaces) auto_ifaces = g_hash_table_new_full (nm_str_hash, g_str_equal, g_free, NULL); g_hash_table_add (auto_ifaces, g_strdup (block->name)); - } else if (!strcmp ("iface", block->type)) { + } else if (nm_streq (block->type, "iface")) { NMIfupdownConnection *exported; /* Bridge configuration */ - if(!strncmp ("br", block->name, 2)) { + if (g_str_has_prefix (block_name, "br")) { /* Try to find bridge ports */ const char *ports = ifparser_getkey (block, "bridge-ports"); if (ports) { @@ -346,21 +346,20 @@ initialize (NMSettingsPlugin *config) for (i = 0; i < g_strv_length (port_ifaces); i++) { char *token = port_ifaces[i]; /* Skip crazy stuff like regex or all */ - if (!strcmp ("all", token)) { + if (nm_streq (token, "all")) { continue; } /* Small SM to skip everything inside regex */ - if (!strcmp ("regex", token)) { + if (nm_streq (token, "regex")) { state++; continue; } - if (!strcmp ("noregex", token)) { + if (nm_streq (token, "noregex")) { state--; continue; } - if (!strcmp ("none", token)) { + if (nm_streq (token, "none")) continue; - } if (state == 0 && strlen (token) > 0) { nm_log_info (LOGD_SETTINGS, "adding bridge port %s to eni_ifaces", token); g_hash_table_add (priv->eni_ifaces, g_strdup (token)); @@ -372,9 +371,8 @@ initialize (NMSettingsPlugin *config) } /* Skip loopback configuration */ - if(!strcmp ("lo", block->name)) { + if (nm_streq (block->name, "lo")) goto next; - } /* Remove any connection for this block that was previously found */ exported = g_hash_table_lookup (priv->connections, block->name); @@ -392,7 +390,7 @@ initialize (NMSettingsPlugin *config) } nm_log_info (LOGD_SETTINGS, "adding iface %s to eni_ifaces", block->name); g_hash_table_add (priv->eni_ifaces, g_strdup (block->name)); - } else if (!strcmp ("mapping", block->type)) { + } else if (nm_streq (block->type, "mapping")) { g_hash_table_add (priv->eni_ifaces, g_strdup (block->name)); nm_log_info (LOGD_SETTINGS, "adding mapping %s to eni_ifaces", block->name); } -- cgit v1.2.1 From f0509205a2ba62036b0c0d4b63f899821e81811e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 25 Aug 2018 10:22:11 +0200 Subject: settings/ifupdown: refactor parsing loop in plugin's initialize() --- .../plugins/ifupdown/nms-ifupdown-plugin.c | 23 ++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c index 564cb83aa9..d6e50dcbb0 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c @@ -322,13 +322,16 @@ initialize (NMSettingsPlugin *config) /* Read in all the interfaces */ ifparser_init (ENI_INTERFACES_FILE, 0); - block = ifparser_getfirst (); - while (block) { + for (block = ifparser_getfirst (); block; block = block->next) { + if (NM_IN_STRSET (block->type, "auto", "allow-hotplug")) { if (!auto_ifaces) auto_ifaces = g_hash_table_new_full (nm_str_hash, g_str_equal, g_free, NULL); g_hash_table_add (auto_ifaces, g_strdup (block->name)); - } else if (nm_streq (block->type, "iface")) { + continue; + } + + if (nm_streq (block->type, "iface")) { NMIfupdownConnection *exported; /* Bridge configuration */ @@ -367,12 +370,12 @@ initialize (NMSettingsPlugin *config) } g_strfreev (port_ifaces); } - goto next; + continue; } /* Skip loopback configuration */ if (nm_streq (block->name, "lo")) - goto next; + continue; /* Remove any connection for this block that was previously found */ exported = g_hash_table_lookup (priv->connections, block->name); @@ -390,12 +393,16 @@ initialize (NMSettingsPlugin *config) } nm_log_info (LOGD_SETTINGS, "adding iface %s to eni_ifaces", block->name); g_hash_table_add (priv->eni_ifaces, g_strdup (block->name)); - } else if (nm_streq (block->type, "mapping")) { + continue; + } + + if (nm_streq (block->type, "mapping")) { g_hash_table_add (priv->eni_ifaces, g_strdup (block->name)); nm_log_info (LOGD_SETTINGS, "adding mapping %s to eni_ifaces", block->name); + continue; } - next: - block = block->next; + + /* unhandled. */ } /* Make 'auto' interfaces autoconnect=TRUE */ -- cgit v1.2.1 From f804b23b64673990b9068f5be03755440acd009b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 25 Aug 2018 10:26:44 +0200 Subject: settings/ifupdown: cleanup parsing bridge in plugin's initialize() --- src/settings/plugins/ifupdown/nms-ifupdown-plugin.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c index d6e50dcbb0..67961c90e3 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c @@ -338,20 +338,22 @@ initialize (NMSettingsPlugin *config) if (g_str_has_prefix (block_name, "br")) { /* Try to find bridge ports */ const char *ports = ifparser_getkey (block, "bridge-ports"); + if (ports) { - int i; + guint i; int state = 0; - char **port_ifaces; + gs_strfreev char **port_ifaces = NULL; nm_log_info (LOGD_SETTINGS, "found bridge ports %s for %s", ports, block->name); port_ifaces = g_strsplit_set (ports, " \t", -1); - for (i = 0; i < g_strv_length (port_ifaces); i++) { - char *token = port_ifaces[i]; + for (i = 0; port_ifaces[i]; i++) { + const char *token = port_ifaces[i]; + /* Skip crazy stuff like regex or all */ - if (nm_streq (token, "all")) { + if (nm_streq (token, "all")) continue; - } + /* Small SM to skip everything inside regex */ if (nm_streq (token, "regex")) { state++; @@ -368,7 +370,6 @@ initialize (NMSettingsPlugin *config) g_hash_table_add (priv->eni_ifaces, g_strdup (token)); } } - g_strfreev (port_ifaces); } continue; } -- cgit v1.2.1 From fb04a7b722224c33d443ccf364117466f7ae5adb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 25 Aug 2018 10:34:46 +0200 Subject: settings/ifupdown: cleanup plugin's get_connections() --- src/settings/plugins/ifupdown/nms-ifupdown-plugin.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c index 67961c90e3..adf72c0a32 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c @@ -243,22 +243,18 @@ handle_uevent (NMUdevClient *client, * list is freed by the system settings service. */ static GSList* -get_connections (NMSettingsPlugin *config) +get_connections (NMSettingsPlugin *plugin) { - SettingsPluginIfupdownPrivate *priv = SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE ((SettingsPluginIfupdown *) config); - GSList *connections; - - nm_log_info (LOGD_SETTINGS, "(%d) ... get_connections.", GPOINTER_TO_UINT(config)); + SettingsPluginIfupdown *self = SETTINGS_PLUGIN_IFUPDOWN (plugin); + SettingsPluginIfupdownPrivate *priv = SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE (self); - if (priv->unmanage_well_known) { - nm_log_info (LOGD_SETTINGS, "(%d) ... get_connections (managed=false): return empty list.", GPOINTER_TO_UINT(config)); + if(priv->unmanage_well_known) { + nm_log_info (LOGD_SETTINGS, "(%d) get_connections: return empty list due to managed=false", GPOINTER_TO_UINT (self)); return NULL; } - connections = _nm_utils_hash_values_to_slist (priv->connections); - - nm_log_info (LOGD_SETTINGS, "(%d) connections count: %d", GPOINTER_TO_UINT(config), g_slist_length(connections)); - return connections; + nm_log_info (LOGD_SETTINGS, "(%d) get_connections: connections count: %u", GPOINTER_TO_UINT (self), g_hash_table_size (priv->connections)); + return _nm_utils_hash_values_to_slist (priv->connections); } /* -- cgit v1.2.1 From fab0d214b72aed919b9524cc532a65834f366bc9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 25 Aug 2018 10:36:51 +0200 Subject: settings/ifupdown: cleanup plugin's logging - use _NMLOG() macro and give logging message a sensible prefix - downgrade logging severity. Most of these messages are not important to warrant or level. - the logging is generally rather bad. Messages like "bind-to-connection: locking wired connection setting" don't indicate which profile is locked to which MAC address. TODO. --- .../plugins/ifupdown/nms-ifupdown-plugin.c | 68 ++++++++++++---------- 1 file changed, 37 insertions(+), 31 deletions(-) diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c index adf72c0a32..a90162b599 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c @@ -95,6 +95,16 @@ NM_DEFINE_SINGLETON_GETTER (SettingsPluginIfupdown, settings_plugin_ifupdown_get /*****************************************************************************/ +#define _NMLOG_PREFIX_NAME "ifupdown" +#define _NMLOG_DOMAIN LOGD_SETTINGS +#define _NMLOG(level, ...) \ + nm_log ((level), _NMLOG_DOMAIN, NULL, NULL, \ + "%s" _NM_UTILS_MACRO_FIRST (__VA_ARGS__), \ + _NMLOG_PREFIX_NAME": " \ + _NM_UTILS_MACRO_REST (__VA_ARGS__)) + +/*****************************************************************************/ + static void bind_device_to_connection (SettingsPluginIfupdown *self, struct udev_device *device, @@ -106,29 +116,29 @@ bind_device_to_connection (SettingsPluginIfupdown *self, iface = udev_device_get_sysname (device); if (!iface) { - nm_log_warn (LOGD_SETTINGS, "failed to get ifname for device."); + _LOGD ("bind-to-connection: failed to get ifname for device."); return; } address = udev_device_get_sysattr_value (device, "address"); if (!address || !address[0]) { - nm_log_warn (LOGD_SETTINGS, "failed to get MAC address for %s", iface); + _LOGD ("bind-to-connection: failed to get MAC address for %s", iface); return; } if (!nm_utils_hwaddr_valid (address, ETH_ALEN)) { - nm_log_warn (LOGD_SETTINGS, "failed to parse MAC address '%s' for %s", - address, iface); + _LOGD ("bind-to-connection: failed to parse MAC address '%s' for %s", + address, iface); return; } s_wired = nm_connection_get_setting_wired (nm_settings_connection_get_connection (NM_SETTINGS_CONNECTION (exported))); s_wifi = nm_connection_get_setting_wireless (nm_settings_connection_get_connection (NM_SETTINGS_CONNECTION (exported))); if (s_wired) { - nm_log_info (LOGD_SETTINGS, "locking wired connection setting"); + _LOGD ("bind-to-connection: locking wired connection setting"); g_object_set (s_wired, NM_SETTING_WIRED_MAC_ADDRESS, address, NULL); } else if (s_wifi) { - nm_log_info (LOGD_SETTINGS, "locking wireless connection setting"); + _LOGD ("bind-to-connection: locking wireless connection setting"); g_object_set (s_wifi, NM_SETTING_WIRELESS_MAC_ADDRESS, address, NULL); } @@ -152,7 +162,7 @@ udev_device_added (SettingsPluginIfupdown *self, struct udev_device *device) if (!iface || !path) return; - nm_log_info (LOGD_SETTINGS, "devices added (path: %s, iface: %s)", path, iface); + _LOGD ("udev: devices added (path: %s, iface: %s)", path, iface); /* if we have a configured connection for this particular iface * we want to either unmanage the device or lock it @@ -160,8 +170,8 @@ udev_device_added (SettingsPluginIfupdown *self, struct udev_device *device) exported = g_hash_table_lookup (priv->connections, iface); if ( !exported && !g_hash_table_contains (priv->eni_ifaces, iface)) { - nm_log_info (LOGD_SETTINGS, "device added (path: %s, iface: %s): no ifupdown configuration found.", - path, iface); + _LOGD ("udev: device added (path: %s, iface: %s): no ifupdown configuration found.", + path, iface); return; } @@ -185,7 +195,7 @@ udev_device_removed (SettingsPluginIfupdown *self, struct udev_device *device) if (!iface || !path) return; - nm_log_info (LOGD_SETTINGS, "devices removed (path: %s, iface: %s)", path, iface); + _LOGD ("udev: devices removed (path: %s, iface: %s)", path, iface); if (!g_hash_table_remove (priv->kernel_ifaces, iface)) return; @@ -205,7 +215,7 @@ udev_device_changed (SettingsPluginIfupdown *self, struct udev_device *device) if (!iface || !path) return; - nm_log_info (LOGD_SETTINGS, "device changed (path: %s, iface: %s)", path, iface); + _LOGD ("udev: device changed (path: %s, iface: %s)", path, iface); if (!g_hash_table_lookup (priv->kernel_ifaces, iface)) return; @@ -249,11 +259,11 @@ get_connections (NMSettingsPlugin *plugin) SettingsPluginIfupdownPrivate *priv = SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE (self); if(priv->unmanage_well_known) { - nm_log_info (LOGD_SETTINGS, "(%d) get_connections: return empty list due to managed=false", GPOINTER_TO_UINT (self)); + _LOGD ("get_connections: not connections due to managed=false"); return NULL; } - nm_log_info (LOGD_SETTINGS, "(%d) get_connections: connections count: %u", GPOINTER_TO_UINT (self), g_hash_table_size (priv->connections)); + _LOGD ("get_connections: %u connections", g_hash_table_size (priv->connections)); return _nm_utils_hash_values_to_slist (priv->connections); } @@ -263,9 +273,10 @@ get_connections (NMSettingsPlugin *plugin) * each element must be allocated using g_malloc() or its variants. */ static GSList* -get_unmanaged_specs (NMSettingsPlugin *config) +get_unmanaged_specs (NMSettingsPlugin *plugin) { - SettingsPluginIfupdownPrivate *priv = SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE ((SettingsPluginIfupdown *) config); + SettingsPluginIfupdown *self = SETTINGS_PLUGIN_IFUPDOWN (plugin); + SettingsPluginIfupdownPrivate *priv = SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE (self); GSList *specs = NULL; GHashTableIter iter; struct udev_device *device; @@ -274,8 +285,8 @@ get_unmanaged_specs (NMSettingsPlugin *config) if (!ALWAYS_UNMANAGE && !priv->unmanage_well_known) return NULL; - nm_log_info (LOGD_SETTINGS, "get unmanaged devices count: %d", - g_hash_table_size (priv->kernel_ifaces)); + _LOGD ("unmanaged-specs: unmanaged devices count %u", + g_hash_table_size (priv->kernel_ifaces)); g_hash_table_iter_init (&iter, priv->kernel_ifaces); while (g_hash_table_iter_next (&iter, (gpointer) &iface, (gpointer) &device)) { @@ -299,9 +310,9 @@ _udev_device_unref (gpointer ptr) } static void -initialize (NMSettingsPlugin *config) +initialize (NMSettingsPlugin *plugin) { - SettingsPluginIfupdown *self = SETTINGS_PLUGIN_IFUPDOWN (config); + SettingsPluginIfupdown *self = SETTINGS_PLUGIN_IFUPDOWN (plugin); SettingsPluginIfupdownPrivate *priv = SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE (self); gs_unref_hashtable GHashTable *auto_ifaces = NULL; if_block *block = NULL; @@ -311,8 +322,6 @@ initialize (NMSettingsPlugin *config) const char *block_name; NMIfupdownConnection *connection; - nm_log_info (LOGD_SETTINGS, "init!"); - priv->udev_client = nm_udev_client_new ((const char *[]) { "net", NULL }, handle_uevent, self); @@ -340,7 +349,7 @@ initialize (NMSettingsPlugin *config) int state = 0; gs_strfreev char **port_ifaces = NULL; - nm_log_info (LOGD_SETTINGS, "found bridge ports %s for %s", ports, block->name); + _LOGD ("found bridge ports %s for %s", ports, block->name); port_ifaces = g_strsplit_set (ports, " \t", -1); for (i = 0; port_ifaces[i]; i++) { @@ -362,7 +371,7 @@ initialize (NMSettingsPlugin *config) if (nm_streq (token, "none")) continue; if (state == 0 && strlen (token) > 0) { - nm_log_info (LOGD_SETTINGS, "adding bridge port %s to eni_ifaces", token); + _LOGD ("adding bridge port %s to eni_ifaces", token); g_hash_table_add (priv->eni_ifaces, g_strdup (token)); } } @@ -377,7 +386,7 @@ initialize (NMSettingsPlugin *config) /* Remove any connection for this block that was previously found */ exported = g_hash_table_lookup (priv->connections, block->name); if (exported) { - nm_log_info (LOGD_SETTINGS, "deleting %s from connections", block->name); + _LOGD ("deleting %s from connections", block->name); nm_settings_connection_delete (NM_SETTINGS_CONNECTION (exported), NULL); g_hash_table_remove (priv->connections, block->name); } @@ -385,17 +394,17 @@ initialize (NMSettingsPlugin *config) /* add the new connection */ exported = nm_ifupdown_connection_new (block); if (exported) { - nm_log_info (LOGD_SETTINGS, "adding %s to connections", block->name); + _LOGD ("adding %s to connections", block->name); g_hash_table_insert (priv->connections, g_strdup (block->name), exported); } - nm_log_info (LOGD_SETTINGS, "adding iface %s to eni_ifaces", block->name); + _LOGD ("adding iface %s to eni_ifaces", block->name); g_hash_table_add (priv->eni_ifaces, g_strdup (block->name)); continue; } if (nm_streq (block->type, "mapping")) { g_hash_table_add (priv->eni_ifaces, g_strdup (block->name)); - nm_log_info (LOGD_SETTINGS, "adding mapping %s to eni_ifaces", block->name); + _LOGD ("adding mapping %s to eni_ifaces", block->name); continue; } @@ -414,7 +423,6 @@ initialize (NMSettingsPlugin *config) /* FIXME(copy-on-write-connection): avoid modifying NMConnection instances and share them via copy-on-write. */ setting = nm_connection_get_setting_connection (nm_settings_connection_get_connection (NM_SETTINGS_CONNECTION (connection))); g_object_set (setting, NM_SETTING_CONNECTION_AUTOCONNECT, TRUE, NULL); - nm_log_info (LOGD_SETTINGS, "autoconnect"); } /* Check the config file to find out whether to manage interfaces */ @@ -422,7 +430,7 @@ initialize (NMSettingsPlugin *config) NM_CONFIG_KEYFILE_GROUP_IFUPDOWN, NM_CONFIG_KEYFILE_KEY_IFUPDOWN_MANAGED, !IFUPDOWN_UNMANAGE_WELL_KNOWN_DEFAULT); - nm_log_info (LOGD_SETTINGS, "management mode: %s", priv->unmanage_well_known ? "unmanaged" : "managed"); + _LOGI ("management mode: %s", priv->unmanage_well_known ? "unmanaged" : "managed"); /* Add well-known interfaces */ enumerate = nm_udev_client_enumerate_new (priv->udev_client); @@ -451,8 +459,6 @@ initialize (NMSettingsPlugin *config) } g_list_free (con_list); } - - nm_log_info (LOGD_SETTINGS, "end _init."); } /*****************************************************************************/ -- cgit v1.2.1 From afb9fa67535f82bad4b2c5bd95ddf3ff1f278bdf Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 25 Aug 2018 11:19:10 +0200 Subject: settings/ifupdown: drop unused define ALWAYS_UNMANAGE in plugin This is only useful for testing. But since the managed flag is configurable via NetworkManager.conf, there is no point in having a define as well. If you want to test it, just configure it. And if you really want to patch the code, then patch "priv->unmanage_well_known" to be always TRUE. --- src/settings/plugins/ifupdown/nms-ifupdown-plugin.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c index a90162b599..4f8741c17c 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c @@ -52,11 +52,6 @@ #define IFUPDOWN_UNMANAGE_WELL_KNOWN_DEFAULT TRUE -/* #define ALWAYS_UNMANAGE TRUE */ -#ifndef ALWAYS_UNMANAGE -#define ALWAYS_UNMANAGE FALSE -#endif - /*****************************************************************************/ typedef struct { @@ -180,7 +175,7 @@ udev_device_added (SettingsPluginIfupdown *self, struct udev_device *device) if (exported) bind_device_to_connection (self, device, exported); - if (ALWAYS_UNMANAGE || priv->unmanage_well_known) + if (priv->unmanage_well_known) _nm_settings_plugin_emit_signal_unmanaged_specs_changed (NM_SETTINGS_PLUGIN (self)); } @@ -200,7 +195,7 @@ udev_device_removed (SettingsPluginIfupdown *self, struct udev_device *device) if (!g_hash_table_remove (priv->kernel_ifaces, iface)) return; - if (ALWAYS_UNMANAGE || priv->unmanage_well_known) + if (priv->unmanage_well_known) _nm_settings_plugin_emit_signal_unmanaged_specs_changed (NM_SETTINGS_PLUGIN (self)); } @@ -220,7 +215,7 @@ udev_device_changed (SettingsPluginIfupdown *self, struct udev_device *device) if (!g_hash_table_lookup (priv->kernel_ifaces, iface)) return; - if (ALWAYS_UNMANAGE || priv->unmanage_well_known) + if (priv->unmanage_well_known) _nm_settings_plugin_emit_signal_unmanaged_specs_changed (NM_SETTINGS_PLUGIN (self)); } @@ -282,7 +277,7 @@ get_unmanaged_specs (NMSettingsPlugin *plugin) struct udev_device *device; const char *iface; - if (!ALWAYS_UNMANAGE && !priv->unmanage_well_known) + if (!priv->unmanage_well_known) return NULL; _LOGD ("unmanaged-specs: unmanaged devices count %u", -- cgit v1.2.1 From dfadaaf7f8047a05fb8f42f085e5e7777f213e59 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 25 Aug 2018 11:23:02 +0200 Subject: settings/ifupdown: change plugin's field @unmanage_well_known to @ifupdown_managed @unmanage_well_known directly depends on the "ifupdown.managed" setting from NetworkManager.conf. Rename it (and invert the meaning) so that this relation ship becomes clearer. Also, the double negation of "if (!unmanaged_well_known)" hurts the brain. --- .../plugins/ifupdown/nms-ifupdown-plugin.c | 24 +++++++++++----------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c index 4f8741c17c..cc7c5431b6 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c @@ -67,7 +67,7 @@ typedef struct { /* Stores any network interfaces the kernel knows about */ GHashTable *kernel_ifaces; - gboolean unmanage_well_known; + bool ifupdown_managed; } SettingsPluginIfupdownPrivate; struct _SettingsPluginIfupdown { @@ -175,7 +175,7 @@ udev_device_added (SettingsPluginIfupdown *self, struct udev_device *device) if (exported) bind_device_to_connection (self, device, exported); - if (priv->unmanage_well_known) + if (!priv->ifupdown_managed) _nm_settings_plugin_emit_signal_unmanaged_specs_changed (NM_SETTINGS_PLUGIN (self)); } @@ -195,7 +195,7 @@ udev_device_removed (SettingsPluginIfupdown *self, struct udev_device *device) if (!g_hash_table_remove (priv->kernel_ifaces, iface)) return; - if (priv->unmanage_well_known) + if (!priv->ifupdown_managed) _nm_settings_plugin_emit_signal_unmanaged_specs_changed (NM_SETTINGS_PLUGIN (self)); } @@ -215,7 +215,7 @@ udev_device_changed (SettingsPluginIfupdown *self, struct udev_device *device) if (!g_hash_table_lookup (priv->kernel_ifaces, iface)) return; - if (priv->unmanage_well_known) + if (!priv->ifupdown_managed) _nm_settings_plugin_emit_signal_unmanaged_specs_changed (NM_SETTINGS_PLUGIN (self)); } @@ -253,7 +253,7 @@ get_connections (NMSettingsPlugin *plugin) SettingsPluginIfupdown *self = SETTINGS_PLUGIN_IFUPDOWN (plugin); SettingsPluginIfupdownPrivate *priv = SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE (self); - if(priv->unmanage_well_known) { + if (!priv->ifupdown_managed) { _LOGD ("get_connections: not connections due to managed=false"); return NULL; } @@ -277,7 +277,7 @@ get_unmanaged_specs (NMSettingsPlugin *plugin) struct udev_device *device; const char *iface; - if (!priv->unmanage_well_known) + if (priv->ifupdown_managed) return NULL; _LOGD ("unmanaged-specs: unmanaged devices count %u", @@ -421,11 +421,11 @@ initialize (NMSettingsPlugin *plugin) } /* Check the config file to find out whether to manage interfaces */ - priv->unmanage_well_known = !nm_config_data_get_value_boolean (NM_CONFIG_GET_DATA_ORIG, - NM_CONFIG_KEYFILE_GROUP_IFUPDOWN, - NM_CONFIG_KEYFILE_KEY_IFUPDOWN_MANAGED, - !IFUPDOWN_UNMANAGE_WELL_KNOWN_DEFAULT); - _LOGI ("management mode: %s", priv->unmanage_well_known ? "unmanaged" : "managed"); + priv->ifupdown_managed = nm_config_data_get_value_boolean (NM_CONFIG_GET_DATA_ORIG, + NM_CONFIG_KEYFILE_GROUP_IFUPDOWN, + NM_CONFIG_KEYFILE_KEY_IFUPDOWN_MANAGED, + !IFUPDOWN_UNMANAGE_WELL_KNOWN_DEFAULT); + _LOGI ("management mode: %s", priv->ifupdown_managed ? "managed" : "unmanaged"); /* Add well-known interfaces */ enumerate = nm_udev_client_enumerate_new (priv->udev_client); @@ -444,7 +444,7 @@ initialize (NMSettingsPlugin *plugin) udev_enumerate_unref (enumerate); /* Now if we're running in managed mode, let NM know there are new connections */ - if (!priv->unmanage_well_known) { + if (priv->ifupdown_managed) { GList *con_list = g_hash_table_get_values (priv->connections); GList *cl_iter; -- cgit v1.2.1 From 6aa66426a4168b3db115646f410bcb5deea6847b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 25 Aug 2018 11:44:24 +0200 Subject: settings/ifupdown: merge eni_ifaces and connections hashes in plugin The "connections" hash contains a mapping of block->name (iface) to the NMSettingsConnection. The "eni_ifaces" hash contains the name of all blocks which are mentioned, but for which no connection was created. Merge the two hashes. We don't need to keep track of whether a connections was successfully created ("connections"), but the same name also has a non-connection block. This information is unnecessary, so one hash is enough. --- .../plugins/ifupdown/nms-ifupdown-plugin.c | 113 ++++++++++++--------- 1 file changed, 63 insertions(+), 50 deletions(-) diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c index cc7c5431b6..92ab732e40 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c @@ -57,10 +57,8 @@ typedef struct { NMUdevClient *udev_client; - GHashTable *connections; /* /e/n/i block name :: NMIfupdownConnection */ - - /* Stores all blocks/interfaces read from /e/n/i regardless of whether - * there is an NMIfupdownConnection for block. + /* Stores an entry for blocks/interfaces read from /e/n/i and (if exists) + * the NMIfupdownConnection associated with the block. */ GHashTable *eni_ifaces; @@ -103,7 +101,7 @@ NM_DEFINE_SINGLETON_GETTER (SettingsPluginIfupdown, settings_plugin_ifupdown_get static void bind_device_to_connection (SettingsPluginIfupdown *self, struct udev_device *device, - NMIfupdownConnection *exported) + NMIfupdownConnection *conn) { NMSettingWired *s_wired; NMSettingWireless *s_wifi; @@ -127,8 +125,8 @@ bind_device_to_connection (SettingsPluginIfupdown *self, return; } - s_wired = nm_connection_get_setting_wired (nm_settings_connection_get_connection (NM_SETTINGS_CONNECTION (exported))); - s_wifi = nm_connection_get_setting_wireless (nm_settings_connection_get_connection (NM_SETTINGS_CONNECTION (exported))); + s_wired = nm_connection_get_setting_wired (nm_settings_connection_get_connection (NM_SETTINGS_CONNECTION (conn))); + s_wifi = nm_connection_get_setting_wireless (nm_settings_connection_get_connection (NM_SETTINGS_CONNECTION (conn))); if (s_wired) { _LOGD ("bind-to-connection: locking wired connection setting"); g_object_set (s_wired, NM_SETTING_WIRED_MAC_ADDRESS, address, NULL); @@ -137,7 +135,7 @@ bind_device_to_connection (SettingsPluginIfupdown *self, g_object_set (s_wifi, NM_SETTING_WIRELESS_MAC_ADDRESS, address, NULL); } - nm_settings_connection_update (NM_SETTINGS_CONNECTION (exported), + nm_settings_connection_update (NM_SETTINGS_CONNECTION (conn), NULL, NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK, NM_SETTINGS_CONNECTION_COMMIT_REASON_NONE, @@ -150,7 +148,7 @@ udev_device_added (SettingsPluginIfupdown *self, struct udev_device *device) { SettingsPluginIfupdownPrivate *priv = SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE (self); const char *iface, *path; - NMIfupdownConnection *exported; + NMIfupdownConnection *conn; iface = udev_device_get_sysname (device); path = udev_device_get_syspath (device); @@ -162,9 +160,7 @@ udev_device_added (SettingsPluginIfupdown *self, struct udev_device *device) /* if we have a configured connection for this particular iface * we want to either unmanage the device or lock it */ - exported = g_hash_table_lookup (priv->connections, iface); - if ( !exported - && !g_hash_table_contains (priv->eni_ifaces, iface)) { + if (!g_hash_table_lookup_extended (priv->eni_ifaces, iface, NULL, (gpointer *) &conn)) { _LOGD ("udev: device added (path: %s, iface: %s): no ifupdown configuration found.", path, iface); return; @@ -172,8 +168,8 @@ udev_device_added (SettingsPluginIfupdown *self, struct udev_device *device) g_hash_table_insert (priv->kernel_ifaces, g_strdup (iface), udev_device_ref (device)); - if (exported) - bind_device_to_connection (self, device, exported); + if (conn) + bind_device_to_connection (self, device, conn); if (!priv->ifupdown_managed) _nm_settings_plugin_emit_signal_unmanaged_specs_changed (NM_SETTINGS_PLUGIN (self)); @@ -252,14 +248,23 @@ get_connections (NMSettingsPlugin *plugin) { SettingsPluginIfupdown *self = SETTINGS_PLUGIN_IFUPDOWN (plugin); SettingsPluginIfupdownPrivate *priv = SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE (self); + GSList *list = NULL; + GHashTableIter iter; + void *value; if (!priv->ifupdown_managed) { _LOGD ("get_connections: not connections due to managed=false"); return NULL; } - _LOGD ("get_connections: %u connections", g_hash_table_size (priv->connections)); - return _nm_utils_hash_values_to_slist (priv->connections); + g_hash_table_iter_init (&iter, priv->eni_ifaces); + while (g_hash_table_iter_next (&iter, NULL, &value)) { + if (value) + list = g_slist_prepend (list, value); + } + + _LOGD ("get_connections: %u connections", g_slist_length (list)); + return list; } /* @@ -315,7 +320,7 @@ initialize (NMSettingsPlugin *plugin) struct udev_list_entry *keys; GHashTableIter con_iter; const char *block_name; - NMIfupdownConnection *connection; + NMIfupdownConnection *conn; priv->udev_client = nm_udev_client_new ((const char *[]) { "net", NULL }, handle_uevent, self); @@ -332,8 +337,6 @@ initialize (NMSettingsPlugin *plugin) } if (nm_streq (block->type, "iface")) { - NMIfupdownConnection *exported; - /* Bridge configuration */ if (g_str_has_prefix (block_name, "br")) { /* Try to find bridge ports */ @@ -344,7 +347,7 @@ initialize (NMSettingsPlugin *plugin) int state = 0; gs_strfreev char **port_ifaces = NULL; - _LOGD ("found bridge ports %s for %s", ports, block->name); + _LOGD ("parse: found bridge ports %s for %s", ports, block->name); port_ifaces = g_strsplit_set (ports, " \t", -1); for (i = 0; port_ifaces[i]; i++) { @@ -366,8 +369,14 @@ initialize (NMSettingsPlugin *plugin) if (nm_streq (token, "none")) continue; if (state == 0 && strlen (token) > 0) { - _LOGD ("adding bridge port %s to eni_ifaces", token); - g_hash_table_add (priv->eni_ifaces, g_strdup (token)); + conn = g_hash_table_lookup (priv->eni_ifaces, block->name); + if (!conn) { + _LOGD ("parse: adding bridge port \"%s\"", token); + g_hash_table_insert (priv->eni_ifaces, g_strdup (token), NULL); + } else { + _LOGD ("parse: adding bridge port \"%s\" (have connection %s)", token, + nm_settings_connection_get_uuid (NM_SETTINGS_CONNECTION (conn))); + } } } } @@ -379,44 +388,51 @@ initialize (NMSettingsPlugin *plugin) continue; /* Remove any connection for this block that was previously found */ - exported = g_hash_table_lookup (priv->connections, block->name); - if (exported) { - _LOGD ("deleting %s from connections", block->name); - nm_settings_connection_delete (NM_SETTINGS_CONNECTION (exported), NULL); - g_hash_table_remove (priv->connections, block->name); + conn = g_hash_table_lookup (priv->eni_ifaces, block->name); + if (conn) { + _LOGD ("parse: replace connection \"%s\" (%s)", + block->name, + nm_settings_connection_get_uuid (NM_SETTINGS_CONNECTION (conn))); + nm_settings_connection_delete (NM_SETTINGS_CONNECTION (conn), NULL); + g_hash_table_remove (priv->eni_ifaces, block->name); } /* add the new connection */ - exported = nm_ifupdown_connection_new (block); - if (exported) { - _LOGD ("adding %s to connections", block->name); - g_hash_table_insert (priv->connections, g_strdup (block->name), exported); - } - _LOGD ("adding iface %s to eni_ifaces", block->name); - g_hash_table_add (priv->eni_ifaces, g_strdup (block->name)); + conn = nm_ifupdown_connection_new (block); + if (conn) { + _LOGD ("parse: adding connection \"%s\" (%s)", block->name, + nm_settings_connection_get_uuid (NM_SETTINGS_CONNECTION (conn))); + } else + _LOGD ("parse: adding place holder for connection \"%s\"", block->name); + g_hash_table_insert (priv->eni_ifaces, g_strdup (block->name), conn); continue; } if (nm_streq (block->type, "mapping")) { - g_hash_table_add (priv->eni_ifaces, g_strdup (block->name)); - _LOGD ("adding mapping %s to eni_ifaces", block->name); + conn = g_hash_table_lookup (priv->eni_ifaces, block->name); + if (!conn) { + _LOGD ("parse: adding mapping \"%s\"", block->name); + g_hash_table_insert (priv->eni_ifaces, g_strdup (block->name), NULL); + } else { + _LOGD ("parse: adding mapping \"%s\" (have connection %s)", block->name, + nm_settings_connection_get_uuid (NM_SETTINGS_CONNECTION (conn))); + } continue; } - - /* unhandled. */ } /* Make 'auto' interfaces autoconnect=TRUE */ - g_hash_table_iter_init (&con_iter, priv->connections); - while (g_hash_table_iter_next (&con_iter, (gpointer) &block_name, (gpointer) &connection)) { + g_hash_table_iter_init (&con_iter, priv->eni_ifaces); + while (g_hash_table_iter_next (&con_iter, (gpointer) &block_name, (gpointer) &conn)) { NMSettingConnection *setting; - if ( !auto_ifaces + if ( !conn + || !auto_ifaces || !g_hash_table_contains (auto_ifaces, block_name)) continue; /* FIXME(copy-on-write-connection): avoid modifying NMConnection instances and share them via copy-on-write. */ - setting = nm_connection_get_setting_connection (nm_settings_connection_get_connection (NM_SETTINGS_CONNECTION (connection))); + setting = nm_connection_get_setting_connection (nm_settings_connection_get_connection (NM_SETTINGS_CONNECTION (conn))); g_object_set (setting, NM_SETTING_CONNECTION_AUTOCONNECT, TRUE, NULL); } @@ -445,14 +461,13 @@ initialize (NMSettingsPlugin *plugin) /* Now if we're running in managed mode, let NM know there are new connections */ if (priv->ifupdown_managed) { - GList *con_list = g_hash_table_get_values (priv->connections); - GList *cl_iter; + GHashTableIter iter; - for (cl_iter = con_list; cl_iter; cl_iter = g_list_next (cl_iter)) { + g_hash_table_iter_init (&iter, priv->eni_ifaces); + while (g_hash_table_iter_next (&iter, NULL, (gpointer *) conn)) { _nm_settings_plugin_emit_signal_connection_added (NM_SETTINGS_PLUGIN (self), - NM_SETTINGS_CONNECTION (cl_iter->data)); + NM_SETTINGS_CONNECTION (conn)); } - g_list_free (con_list); } } @@ -463,9 +478,8 @@ settings_plugin_ifupdown_init (SettingsPluginIfupdown *self) { SettingsPluginIfupdownPrivate *priv = SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE (self); - priv->connections = g_hash_table_new_full (nm_str_hash, g_str_equal, g_free, g_object_unref); + priv->eni_ifaces = g_hash_table_new_full (nm_str_hash, g_str_equal, g_free, g_object_unref); priv->kernel_ifaces = g_hash_table_new_full (nm_str_hash, g_str_equal, g_free, _udev_device_unref); - priv->eni_ifaces = g_hash_table_new_full (nm_str_hash, g_str_equal, g_free, NULL); } static void @@ -474,7 +488,6 @@ dispose (GObject *object) SettingsPluginIfupdown *plugin = SETTINGS_PLUGIN_IFUPDOWN (object); SettingsPluginIfupdownPrivate *priv = SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE (plugin); - g_clear_pointer (&priv->connections, g_hash_table_destroy); g_clear_pointer (&priv->kernel_ifaces, g_hash_table_destroy); g_clear_pointer (&priv->eni_ifaces, g_hash_table_destroy); -- cgit v1.2.1 From 518c7be77b63f573fd7f928ee67fc4814918feb7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 25 Aug 2018 12:11:27 +0200 Subject: settings/ifupdown: in plugin drop listening to udev for devices Don't listen to udev to find out about devices. First of all, using udev for this is already very wrong, because we have the platform cache. Anyway, all that the device information is used, is pointless as well. Drop it. It's pointless because: The entires in eni_ifaces are already indexed by the interface name. Likewise, all NMIfupdownConnection set "connection.interface-name" to restict the profile by name. /e/n/i matches devices is by name, that's it. We don't need udev to look up the MAC address (by name!!) to later ignore/match devices by MAC address. Especially, because NetworkMaanger can already restrict profiles to devices based on the interface name. Likewise, NetworkMaanger can use the interface name for the unmanaged-specs. It's wrong to extend the on-disk configuration from /e/n/i with runtime information from udev, especially, because other NetworkMaanger layers are perfectly content using the interface name for this purpose. Also, bind_device_to_connection() was fundamentally wrong. It's wrong to modify the settings connection at random moments (on udev event). If at all, that should only happen during connection load/reload. This may have been necessary a long time ago, when unmanaged devices were not expressed by device-match-specs, but by the HAL UDI. That was since improved, for example by commit c9067d8fedf6f6f2d530fd68bbfca7ce68638d38. --- .../plugins/ifupdown/nms-ifupdown-plugin.c | 196 +-------------------- 1 file changed, 4 insertions(+), 192 deletions(-) diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c index 92ab732e40..07bf7ae985 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c @@ -29,7 +29,6 @@ #include #include #include -#include #include "nm-setting-connection.h" #include "nm-dbus-interface.h" @@ -42,7 +41,6 @@ #include "nm-core-internal.h" #include "NetworkManagerUtils.h" #include "nm-config.h" -#include "nm-utils/nm-udev-utils.h" #include "nms-ifupdown-interface-parser.h" #include "nms-ifupdown-connection.h" @@ -55,16 +53,11 @@ /*****************************************************************************/ typedef struct { - NMUdevClient *udev_client; - /* Stores an entry for blocks/interfaces read from /e/n/i and (if exists) * the NMIfupdownConnection associated with the block. */ GHashTable *eni_ifaces; - /* Stores any network interfaces the kernel knows about */ - GHashTable *kernel_ifaces; - bool ifupdown_managed; } SettingsPluginIfupdownPrivate; @@ -98,148 +91,6 @@ NM_DEFINE_SINGLETON_GETTER (SettingsPluginIfupdown, settings_plugin_ifupdown_get /*****************************************************************************/ -static void -bind_device_to_connection (SettingsPluginIfupdown *self, - struct udev_device *device, - NMIfupdownConnection *conn) -{ - NMSettingWired *s_wired; - NMSettingWireless *s_wifi; - const char *iface, *address; - - iface = udev_device_get_sysname (device); - if (!iface) { - _LOGD ("bind-to-connection: failed to get ifname for device."); - return; - } - - address = udev_device_get_sysattr_value (device, "address"); - if (!address || !address[0]) { - _LOGD ("bind-to-connection: failed to get MAC address for %s", iface); - return; - } - - if (!nm_utils_hwaddr_valid (address, ETH_ALEN)) { - _LOGD ("bind-to-connection: failed to parse MAC address '%s' for %s", - address, iface); - return; - } - - s_wired = nm_connection_get_setting_wired (nm_settings_connection_get_connection (NM_SETTINGS_CONNECTION (conn))); - s_wifi = nm_connection_get_setting_wireless (nm_settings_connection_get_connection (NM_SETTINGS_CONNECTION (conn))); - if (s_wired) { - _LOGD ("bind-to-connection: locking wired connection setting"); - g_object_set (s_wired, NM_SETTING_WIRED_MAC_ADDRESS, address, NULL); - } else if (s_wifi) { - _LOGD ("bind-to-connection: locking wireless connection setting"); - g_object_set (s_wifi, NM_SETTING_WIRELESS_MAC_ADDRESS, address, NULL); - } - - nm_settings_connection_update (NM_SETTINGS_CONNECTION (conn), - NULL, - NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK, - NM_SETTINGS_CONNECTION_COMMIT_REASON_NONE, - "ifupdown-new", - NULL); -} - -static void -udev_device_added (SettingsPluginIfupdown *self, struct udev_device *device) -{ - SettingsPluginIfupdownPrivate *priv = SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE (self); - const char *iface, *path; - NMIfupdownConnection *conn; - - iface = udev_device_get_sysname (device); - path = udev_device_get_syspath (device); - if (!iface || !path) - return; - - _LOGD ("udev: devices added (path: %s, iface: %s)", path, iface); - - /* if we have a configured connection for this particular iface - * we want to either unmanage the device or lock it - */ - if (!g_hash_table_lookup_extended (priv->eni_ifaces, iface, NULL, (gpointer *) &conn)) { - _LOGD ("udev: device added (path: %s, iface: %s): no ifupdown configuration found.", - path, iface); - return; - } - - g_hash_table_insert (priv->kernel_ifaces, g_strdup (iface), udev_device_ref (device)); - - if (conn) - bind_device_to_connection (self, device, conn); - - if (!priv->ifupdown_managed) - _nm_settings_plugin_emit_signal_unmanaged_specs_changed (NM_SETTINGS_PLUGIN (self)); -} - -static void -udev_device_removed (SettingsPluginIfupdown *self, struct udev_device *device) -{ - SettingsPluginIfupdownPrivate *priv = SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE (self); - const char *iface, *path; - - iface = udev_device_get_sysname (device); - path = udev_device_get_syspath (device); - if (!iface || !path) - return; - - _LOGD ("udev: devices removed (path: %s, iface: %s)", path, iface); - - if (!g_hash_table_remove (priv->kernel_ifaces, iface)) - return; - - if (!priv->ifupdown_managed) - _nm_settings_plugin_emit_signal_unmanaged_specs_changed (NM_SETTINGS_PLUGIN (self)); -} - -static void -udev_device_changed (SettingsPluginIfupdown *self, struct udev_device *device) -{ - SettingsPluginIfupdownPrivate *priv = SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE (self); - const char *iface, *path; - - iface = udev_device_get_sysname (device); - path = udev_device_get_syspath (device); - if (!iface || !path) - return; - - _LOGD ("udev: device changed (path: %s, iface: %s)", path, iface); - - if (!g_hash_table_lookup (priv->kernel_ifaces, iface)) - return; - - if (!priv->ifupdown_managed) - _nm_settings_plugin_emit_signal_unmanaged_specs_changed (NM_SETTINGS_PLUGIN (self)); -} - -static void -handle_uevent (NMUdevClient *client, - struct udev_device *device, - gpointer user_data) -{ - SettingsPluginIfupdown *self = SETTINGS_PLUGIN_IFUPDOWN (user_data); - const char *subsys; - const char *action; - - action = udev_device_get_action (device); - - g_return_if_fail (action != NULL); - - /* A bit paranoid */ - subsys = udev_device_get_subsystem (device); - g_return_if_fail (nm_streq0 (subsys, "net")); - - if (nm_streq (action, "add")) - udev_device_added (self, device); - else if (nm_streq (action, "remove")) - udev_device_removed (self, device); - else if (nm_streq (action, "change")) - udev_device_changed (self, device); -} - /* Returns the plugins currently known list of connections. The returned * list is freed by the system settings service. */ @@ -279,36 +130,22 @@ get_unmanaged_specs (NMSettingsPlugin *plugin) SettingsPluginIfupdownPrivate *priv = SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE (self); GSList *specs = NULL; GHashTableIter iter; - struct udev_device *device; const char *iface; if (priv->ifupdown_managed) return NULL; _LOGD ("unmanaged-specs: unmanaged devices count %u", - g_hash_table_size (priv->kernel_ifaces)); + g_hash_table_size (priv->eni_ifaces)); - g_hash_table_iter_init (&iter, priv->kernel_ifaces); - while (g_hash_table_iter_next (&iter, (gpointer) &iface, (gpointer) &device)) { - const char *address; - - address = udev_device_get_sysattr_value (device, "address"); - if (address) - specs = g_slist_append (specs, g_strdup_printf ("mac:%s", address)); - else - specs = g_slist_append (specs, g_strdup_printf ("interface-name:%s", iface)); - } + g_hash_table_iter_init (&iter, priv->eni_ifaces); + while (g_hash_table_iter_next (&iter, (gpointer) &iface, NULL)) + specs = g_slist_append (specs, g_strdup_printf ("interface-name:=%s", iface)); return specs; } /*****************************************************************************/ -static void -_udev_device_unref (gpointer ptr) -{ - udev_device_unref (ptr); -} - static void initialize (NMSettingsPlugin *plugin) { @@ -316,15 +153,10 @@ initialize (NMSettingsPlugin *plugin) SettingsPluginIfupdownPrivate *priv = SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE (self); gs_unref_hashtable GHashTable *auto_ifaces = NULL; if_block *block = NULL; - struct udev_enumerate *enumerate; - struct udev_list_entry *keys; GHashTableIter con_iter; const char *block_name; NMIfupdownConnection *conn; - priv->udev_client = nm_udev_client_new ((const char *[]) { "net", NULL }, - handle_uevent, self); - /* Read in all the interfaces */ ifparser_init (ENI_INTERFACES_FILE, 0); for (block = ifparser_getfirst (); block; block = block->next) { @@ -443,22 +275,6 @@ initialize (NMSettingsPlugin *plugin) !IFUPDOWN_UNMANAGE_WELL_KNOWN_DEFAULT); _LOGI ("management mode: %s", priv->ifupdown_managed ? "managed" : "unmanaged"); - /* Add well-known interfaces */ - enumerate = nm_udev_client_enumerate_new (priv->udev_client); - udev_enumerate_scan_devices (enumerate); - keys = udev_enumerate_get_list_entry (enumerate); - for (; keys; keys = udev_list_entry_get_next (keys)) { - struct udev_device *udevice; - - udevice = udev_device_new_from_syspath (udev_enumerate_get_udev (enumerate), - udev_list_entry_get_name (keys)); - if (udevice) { - udev_device_added (self, udevice); - udev_device_unref (udevice); - } - } - udev_enumerate_unref (enumerate); - /* Now if we're running in managed mode, let NM know there are new connections */ if (priv->ifupdown_managed) { GHashTableIter iter; @@ -479,7 +295,6 @@ settings_plugin_ifupdown_init (SettingsPluginIfupdown *self) SettingsPluginIfupdownPrivate *priv = SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE (self); priv->eni_ifaces = g_hash_table_new_full (nm_str_hash, g_str_equal, g_free, g_object_unref); - priv->kernel_ifaces = g_hash_table_new_full (nm_str_hash, g_str_equal, g_free, _udev_device_unref); } static void @@ -488,11 +303,8 @@ dispose (GObject *object) SettingsPluginIfupdown *plugin = SETTINGS_PLUGIN_IFUPDOWN (object); SettingsPluginIfupdownPrivate *priv = SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE (plugin); - g_clear_pointer (&priv->kernel_ifaces, g_hash_table_destroy); g_clear_pointer (&priv->eni_ifaces, g_hash_table_destroy); - priv->udev_client = nm_udev_client_unref (priv->udev_client); - G_OBJECT_CLASS (settings_plugin_ifupdown_parent_class)->dispose (object); } -- cgit v1.2.1 From 03be91f038b5f34c48db0e1092c425155a821d48 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 25 Aug 2018 13:14:49 +0200 Subject: settings/ifupdown: adjust coding style for "nms-ifupdown-interface-parser" --- .../ifupdown/nms-ifupdown-interface-parser.c | 138 ++++++------- .../ifupdown/nms-ifupdown-interface-parser.h | 31 ++- .../plugins/ifupdown/nms-ifupdown-parser.c | 216 ++++++++++----------- 3 files changed, 193 insertions(+), 192 deletions(-) diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c b/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c index 94a65ecb9e..178acb3039 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c @@ -34,25 +34,25 @@ if_block* first; if_block* last; - if_data* last_data; -void add_block(const char *type, const char* name) +void +add_block (const char *type, const char* name) { if_block *ret = g_slice_new0 (struct _if_block); - ret->name = g_strdup(name); - ret->type = g_strdup(type); + ret->name = g_strdup (name); + ret->type = g_strdup (type); if (first == NULL) first = last = ret; - else - { + else { last->next = ret; last = ret; } last_data = NULL; } -void add_data(const char *key,const char *data) +void +add_data (const char *key, const char *data) { if_data *ret; char *idx; @@ -62,42 +62,40 @@ void add_data(const char *key,const char *data) return; ret = g_slice_new0 (struct _if_data); - ret->key = g_strdup(key); + ret->key = g_strdup (key); /* Normalize keys. Convert '_' to '-', as ifupdown accepts both variants. * When querying keys via ifparser_getkey(), use '-'. */ - while ((idx = strrchr(ret->key, '_'))) { + while ((idx = strrchr (ret->key, '_'))) { *idx = '-'; } - ret->data = g_strdup(data); + ret->data = g_strdup (data); - if (last->info == NULL) - { + if (last->info == NULL) { last->info = ret; last_data = ret; - } - else - { + } else { last_data->next = ret; last_data = last_data->next; } } /* join values in src with spaces into dst; dst needs to be large enough */ -static char *join_values_with_spaces(char *dst, char **src) +static char * +join_values_with_spaces (char *dst, char **src) { if (dst != NULL) { *dst = '\0'; if (src != NULL && *src != NULL) { - strcat(dst, *src); + strcat (dst, *src); for (src++; *src != NULL; src++) { - strcat(dst, " "); - strcat(dst, *src); + strcat (dst, " "); + strcat (dst, *src); } } } - return(dst); + return (dst); } static void _ifparser_source (const char *path, const char *en_dir, int quiet, int dir); @@ -126,21 +124,20 @@ _recursive_ifparser (const char *eni_file, int quiet) if (!quiet) nm_log_info (LOGD_SETTINGS, " interface-parser: parsing file %s\n", eni_file); - while (!feof(inp)) - { + while (!feof (inp)) { char *token[128]; /* 255 chars can only be split into 127 tokens */ char value[255]; /* large enough to join previously split tokens */ char *safeptr; int toknum; int len = 0; - char *ptr = fgets(line+offs, 255-offs, inp); + char *ptr = fgets (line+offs, 255-offs, inp); if (ptr == NULL) break; - len = strlen(line); + len = strlen (line); /* skip over-long lines */ - if (!feof(inp) && len > 0 && line[len-1] != '\n') { + if (!feof (inp) && len > 0 && line[len-1] != '\n') { if (!skip_long_line) { if (!quiet) nm_log_warn (LOGD_SETTINGS, "Skipping over-long-line '%s...'\n", line); @@ -170,9 +167,9 @@ _recursive_ifparser (const char *eni_file, int quiet) #define SPACES " \t" /* tokenize input; */ - for (toknum = 0, token[toknum] = strtok_r(line, SPACES, &safeptr); + for (toknum = 0, token[toknum] = strtok_r (line, SPACES, &safeptr); token[toknum] != NULL; - toknum++, token[toknum] = strtok_r(NULL, SPACES, &safeptr)) + toknum++, token[toknum] = strtok_r (NULL, SPACES, &safeptr)) ; /* ignore comments and empty lines */ @@ -182,7 +179,7 @@ _recursive_ifparser (const char *eni_file, int quiet) if (toknum < 2) { if (!quiet) { nm_log_warn (LOGD_SETTINGS, "Can't parse interface line '%s'\n", - join_values_with_spaces(value, token)); + join_values_with_spaces (value, token)); } skip_to_block = 1; continue; @@ -193,36 +190,36 @@ _recursive_ifparser (const char *eni_file, int quiet) * Create a block for each of them except source and source-directory. */ /* iface stanza takes at least 3 parameters */ - if (strcmp(token[0], "iface") == 0) { + if (strcmp (token[0], "iface") == 0) { if (toknum < 4) { if (!quiet) { nm_log_warn (LOGD_SETTINGS, "Can't parse iface line '%s'\n", - join_values_with_spaces(value, token)); + join_values_with_spaces (value, token)); } continue; } - add_block(token[0], token[1]); + add_block (token[0], token[1]); skip_to_block = 0; - add_data(token[2], join_values_with_spaces(value, token + 3)); + add_data (token[2], join_values_with_spaces (value, token + 3)); } /* auto and allow-auto stanzas are equivalent, * both can take multiple interfaces as parameters: add one block for each */ - else if (strcmp(token[0], "auto") == 0 || - strcmp(token[0], "allow-auto") == 0) { + else if (strcmp (token[0], "auto") == 0 || + strcmp (token[0], "allow-auto") == 0) { int i; for (i = 1; i < toknum; i++) - add_block("auto", token[i]); + add_block ("auto", token[i]); skip_to_block = 0; } - else if (strcmp(token[0], "mapping") == 0) { - add_block(token[0], join_values_with_spaces(value, token + 1)); + else if (strcmp (token[0], "mapping") == 0) { + add_block (token[0], join_values_with_spaces (value, token + 1)); skip_to_block = 0; } /* allow-* can take multiple interfaces as parameters: add one block for each */ - else if (strncmp(token[0],"allow-",6) == 0) { + else if (strncmp (token[0],"allow-",6) == 0) { int i; for (i = 1; i < toknum; i++) - add_block(token[0], token[i]); + add_block (token[0], token[i]); skip_to_block = 0; } /* source and source-directory stanzas take one or more paths as parameters */ @@ -244,13 +241,13 @@ _recursive_ifparser (const char *eni_file, int quiet) if (skip_to_block) { if (!quiet) { nm_log_warn (LOGD_SETTINGS, "ignoring out-of-block data '%s'\n", - join_values_with_spaces(value, token)); + join_values_with_spaces (value, token)); } } else - add_data(token[0], join_values_with_spaces(value, token + 1)); + add_data (token[0], join_values_with_spaces (value, token + 1)); } } - fclose(inp); + fclose (inp); if (!quiet) nm_log_info (LOGD_SETTINGS, " interface-parser: finished parsing file %s\n", eni_file); @@ -301,47 +298,51 @@ _ifparser_source (const char *path, const char *en_dir, int quiet, int dir) g_free (abs_path); } -void ifparser_init (const char *eni_file, int quiet) +void +ifparser_init (const char *eni_file, int quiet) { first = last = NULL; _recursive_ifparser (eni_file, quiet); } -void _destroy_data(if_data *ifd) +void +_destroy_data (if_data *ifd) { if (ifd == NULL) return; - _destroy_data(ifd->next); - g_free(ifd->key); - g_free(ifd->data); - g_slice_free(struct _if_data, ifd); + _destroy_data (ifd->next); + g_free (ifd->key); + g_free (ifd->data); + g_slice_free (struct _if_data, ifd); return; } -void _destroy_block(if_block* ifb) +void +_destroy_block (if_block* ifb) { if (ifb == NULL) return; - _destroy_block(ifb->next); - _destroy_data(ifb->info); - g_free(ifb->name); - g_free(ifb->type); - g_slice_free(struct _if_block, ifb); + _destroy_block (ifb->next); + _destroy_data (ifb->info); + g_free (ifb->name); + g_free (ifb->type); + g_slice_free (struct _if_block, ifb); return; } -void ifparser_destroy(void) +void +ifparser_destroy (void) { - _destroy_block(first); + _destroy_block (first); first = last = NULL; } -if_block *ifparser_getfirst(void) +if_block *ifparser_getfirst (void) { return first; } -int ifparser_get_num_blocks(void) +int ifparser_get_num_blocks (void) { int i = 0; if_block *iter = first; @@ -353,24 +354,24 @@ int ifparser_get_num_blocks(void) return i; } -if_block *ifparser_getif(const char* iface) +if_block * +ifparser_getif (const char* iface) { if_block *curr = first; - while(curr!=NULL) - { - if (strcmp(curr->type,"iface")==0 && strcmp(curr->name,iface)==0) + while (curr != NULL) { + if (strcmp (curr->type,"iface")==0 && strcmp (curr->name,iface)==0) return curr; curr = curr->next; } return NULL; } -const char *ifparser_getkey(if_block* iface, const char *key) +const char * +ifparser_getkey (if_block* iface, const char *key) { if_data *curr = iface->info; - while(curr!=NULL) - { - if (strcmp(curr->key,key)==0) + while (curr != NULL) { + if (strcmp (curr->key,key)==0) return curr->data; curr = curr->next; } @@ -378,7 +379,7 @@ const char *ifparser_getkey(if_block* iface, const char *key) } gboolean -ifparser_haskey(if_block* iface, const char *key) +ifparser_haskey (if_block* iface, const char *key) { if_data *curr = iface->info; @@ -390,7 +391,8 @@ ifparser_haskey(if_block* iface, const char *key) return FALSE; } -int ifparser_get_num_info(if_block* iface) +int +ifparser_get_num_info (if_block* iface) { int i = 0; if_data *iter = iface->info; diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.h b/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.h index 7e6c8e3471..62b658363f 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.h +++ b/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.h @@ -23,33 +23,32 @@ #ifndef _INTERFACE_PARSER_H #define _INTERFACE_PARSER_H -typedef struct _if_data -{ +typedef struct _if_data { char *key; char *data; struct _if_data *next; } if_data; -typedef struct _if_block -{ +typedef struct _if_block { char *type; char *name; if_data *info; struct _if_block *next; } if_block; -void ifparser_init(const char *eni_file, int quiet); -void ifparser_destroy(void); +void ifparser_init (const char *eni_file, int quiet); +void ifparser_destroy (void); -if_block *ifparser_getif(const char* iface); -if_block *ifparser_getfirst(void); -const char *ifparser_getkey(if_block* iface, const char *key); -gboolean ifparser_haskey(if_block* iface, const char *key); -int ifparser_get_num_blocks(void); -int ifparser_get_num_info(if_block* iface); +if_block *ifparser_getif (const char* iface); +if_block *ifparser_getfirst (void); +const char *ifparser_getkey (if_block* iface, const char *key); +gboolean ifparser_haskey (if_block* iface, const char *key); +int ifparser_get_num_blocks (void); +int ifparser_get_num_info (if_block* iface); + +void add_block (const char *type, const char* name); +void add_data (const char *key,const char *data); +void _destroy_data (if_data *ifd); +void _destroy_block (if_block* ifb); -void add_block(const char *type, const char* name); -void add_data(const char *key,const char *data); -void _destroy_data(if_data *ifd); -void _destroy_block(if_block* ifb); #endif diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-parser.c b/src/settings/plugins/ifupdown/nms-ifupdown-parser.c index a20f79c1ff..4dd922b4a2 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-parser.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-parser.c @@ -42,20 +42,20 @@ _ifupdownplugin_guess_connection_type (if_block *block) { if_data *curr = block->info; const char* ret_type = NULL; - const char* value = ifparser_getkey(block, "inet"); - if(value && !strcmp("ppp", value)) { + const char* value = ifparser_getkey (block, "inet"); + if (value && !strcmp ("ppp", value)) { ret_type = NM_SETTING_PPP_SETTING_NAME; } - while(!ret_type && curr) { - if(!strncmp("wireless-", curr->key, strlen("wireless-")) || - !strncmp("wpa-", curr->key, strlen("wpa-"))) { + while (!ret_type && curr) { + if (!strncmp ("wireless-", curr->key, strlen ("wireless-")) || + !strncmp ("wpa-", curr->key, strlen ("wpa-"))) { ret_type = NM_SETTING_WIRELESS_SETTING_NAME; } curr = curr->next; } - if(!ret_type) + if (!ret_type) ret_type = NM_SETTING_WIRED_SETTING_NAME; nm_log_info (LOGD_SETTINGS, "guessed connection type (%s) = %s", block->name, ret_type); @@ -68,11 +68,11 @@ struct _Mapping { }; static gpointer -map_by_mapping(struct _Mapping *mapping, const char *key) +map_by_mapping (struct _Mapping *mapping, const char *key) { struct _Mapping *curr = mapping; - while(curr->domain) { - if(!strcmp(curr->domain, key)) + while (curr->domain) { + if (!strcmp (curr->domain, key)) return curr->target; curr++; } @@ -80,11 +80,11 @@ map_by_mapping(struct _Mapping *mapping, const char *key) } static void -update_wireless_setting_from_if_block(NMConnection *connection, - if_block *block) +update_wireless_setting_from_if_block (NMConnection *connection, + if_block *block) { - int wpa_l= strlen("wpa-"); - int wireless_l= strlen("wireless-"); + int wpa_l= strlen ("wpa-"); + int wireless_l= strlen ("wireless-"); if_data *curr = block->info; const char* value = ifparser_getkey (block, "inet"); @@ -97,27 +97,27 @@ update_wireless_setting_from_if_block(NMConnection *connection, NMSettingWireless *wireless_setting = NULL; - if(value && !strcmp("ppp", value)) { + if (value && !strcmp ("ppp", value)) { return; } nm_log_info (LOGD_SETTINGS, "update wireless settings (%s).", block->name); - wireless_setting = NM_SETTING_WIRELESS(nm_setting_wireless_new()); + wireless_setting = NM_SETTING_WIRELESS (nm_setting_wireless_new ()); - while(curr) { - if(strlen(curr->key) > wireless_l && - !strncmp("wireless-", curr->key, wireless_l)) { - const char* newkey = map_by_mapping(mapping, curr->key+wireless_l); + while (curr) { + if (strlen (curr->key) > wireless_l && + !strncmp ("wireless-", curr->key, wireless_l)) { + const char* newkey = map_by_mapping (mapping, curr->key+wireless_l); nm_log_info (LOGD_SETTINGS, "wireless setting key: %s='%s'", newkey, curr->data); - if(newkey && !strcmp("ssid", newkey)) { + if (newkey && !strcmp ("ssid", newkey)) { GBytes *ssid; - int len = strlen(curr->data); + int len = strlen (curr->data); ssid = g_bytes_new (curr->data, len); g_object_set (wireless_setting, NM_SETTING_WIRELESS_SSID, ssid, NULL); g_bytes_unref (ssid); nm_log_info (LOGD_SETTINGS, "setting wireless ssid = %d", len); - } else if(newkey && !strcmp("mode", newkey)) { + } else if (newkey && !strcmp ("mode", newkey)) { if (!g_ascii_strcasecmp (curr->data, "Managed") || !g_ascii_strcasecmp (curr->data, "Auto")) g_object_set (wireless_setting, NM_SETTING_WIRELESS_MODE, NM_SETTING_WIRELESS_MODE_INFRA, NULL); else if (!g_ascii_strcasecmp (curr->data, "Ad-Hoc")) @@ -127,33 +127,33 @@ update_wireless_setting_from_if_block(NMConnection *connection, else nm_log_warn (LOGD_SETTINGS, "Invalid mode '%s' (not 'Ad-Hoc', 'Ap', 'Managed', or 'Auto')", curr->data); } else { - g_object_set(wireless_setting, - newkey, curr->data, - NULL); + g_object_set (wireless_setting, + newkey, curr->data, + NULL); } - } else if(strlen(curr->key) > wpa_l && - !strncmp("wpa-", curr->key, wpa_l)) { - const char* newkey = map_by_mapping(mapping, curr->key+wpa_l); + } else if ( strlen (curr->key) > wpa_l + && !strncmp ("wpa-", curr->key, wpa_l)) { + const char* newkey = map_by_mapping (mapping, curr->key+wpa_l); - if(newkey && !strcmp("ssid", newkey)) { + if (newkey && !strcmp ("ssid", newkey)) { GBytes *ssid; - int len = strlen(curr->data); + int len = strlen (curr->data); ssid = g_bytes_new (curr->data, len); g_object_set (wireless_setting, NM_SETTING_WIRELESS_SSID, ssid, NULL); g_bytes_unref (ssid); nm_log_info (LOGD_SETTINGS, "setting wpa ssid = %d", len); - } else if(newkey) { + } else if (newkey) { - g_object_set(wireless_setting, - newkey, curr->data, - NULL); + g_object_set (wireless_setting, + newkey, curr->data, + NULL); nm_log_info (LOGD_SETTINGS, "setting wpa newkey(%s)=data(%s)", newkey, curr->data); } } curr = curr->next; } - nm_connection_add_setting(connection, (NMSetting*) wireless_setting); + nm_connection_add_setting (connection, (NMSetting*) wireless_setting); } typedef char* (*IfupdownStrDupeFunc) (gpointer value, gpointer data); @@ -177,7 +177,7 @@ normalize_dupe_wireless_key (gpointer value, gpointer data) { result_cur += next - delim; delim = next + 1; } - if (*delim && strlen (valuec) > GPOINTER_TO_UINT(delim - valuec)) { + if (*delim && strlen (valuec) > GPOINTER_TO_UINT (delim - valuec)) { strncpy (result_cur, delim, endc - delim); result_cur += endc - delim; } @@ -187,12 +187,12 @@ normalize_dupe_wireless_key (gpointer value, gpointer data) { static char* normalize_dupe (gpointer value, gpointer data) { - return g_strdup(value); + return g_strdup (value); } static char* normalize_tolower (gpointer value, gpointer data) { - return g_ascii_strdown(value, -1); + return g_ascii_strdown (value, -1); } static char *normalize_psk (gpointer value, gpointer data) @@ -203,25 +203,25 @@ static char *normalize_psk (gpointer value, gpointer data) } static gpointer -string_to_gpointerint(const char* data) +string_to_gpointerint (const char* data) { int result = (int) strtol (data, NULL, 10); - return GINT_TO_POINTER(result); + return GINT_TO_POINTER (result); } static gpointer -string_to_glist_of_strings(const char* data) +string_to_glist_of_strings (const char* data) { GSList *ret = NULL; char *string = (char*) data; - while(string) { + while (string) { char* next = NULL; - if( (next = strchr(string, ' ')) || - (next = strchr(string, '\t')) || - (next = strchr(string, '\0')) ) { + if ( (next = strchr (string, ' ')) || + (next = strchr (string, '\t')) || + (next = strchr (string, '\0')) ) { - char *part = g_strndup(string, (next - string)); - ret = g_slist_append(ret, part); + char *part = g_strndup (string, (next - string)); + ret = g_slist_append (ret, part); if (*next) string = next+1; else @@ -234,17 +234,17 @@ string_to_glist_of_strings(const char* data) } static void -slist_free_all(gpointer slist) +slist_free_all (gpointer slist) { g_slist_free_full ((GSList *) slist, g_free); } static void -update_wireless_security_setting_from_if_block(NMConnection *connection, - if_block *block) +update_wireless_security_setting_from_if_block (NMConnection *connection, + if_block *block) { - int wpa_l= strlen("wpa-"); - int wireless_l= strlen("wireless-"); + int wpa_l= strlen ("wpa-"); + int wireless_l= strlen ("wireless-"); if_data *curr = block->info; const char* value = ifparser_getkey (block, "inet"); struct _Mapping mapping[] = { @@ -302,28 +302,27 @@ update_wireless_security_setting_from_if_block(NMConnection *connection, NMSettingWireless *s_wireless; gboolean security = FALSE; - if(value && !strcmp("ppp", value)) { + if (value && !strcmp ("ppp", value)) { return; } - s_wireless = nm_connection_get_setting_wireless(connection); - g_return_if_fail(s_wireless); + s_wireless = nm_connection_get_setting_wireless (connection); + g_return_if_fail (s_wireless); nm_log_info (LOGD_SETTINGS, "update wireless security settings (%s).", block->name); - wireless_security_setting = - NM_SETTING_WIRELESS_SECURITY(nm_setting_wireless_security_new()); + wireless_security_setting = NM_SETTING_WIRELESS_SECURITY (nm_setting_wireless_security_new ()); - while(curr) { - if(strlen(curr->key) > wireless_l && - !strncmp("wireless-", curr->key, wireless_l)) { + while (curr) { + if ( strlen (curr->key) > wireless_l + && !strncmp ("wireless-", curr->key, wireless_l)) { char *property_value = NULL; gpointer typed_property_value = NULL; - const char* newkey = map_by_mapping(mapping, curr->key+wireless_l); + const char* newkey = map_by_mapping (mapping, curr->key+wireless_l); IfupdownStrDupeFunc dupe_func = map_by_mapping (dupe_mapping, curr->key+wireless_l); IfupdownStrToTypeFunc type_map_func = map_by_mapping (type_mapping, curr->key+wireless_l); GFreeFunc free_func = map_by_mapping (free_type_mapping, curr->key+wireless_l); - if(!newkey || !dupe_func) + if (!newkey || !dupe_func) goto next; property_value = (*dupe_func) (curr->data, connection); @@ -333,30 +332,30 @@ update_wireless_security_setting_from_if_block(NMConnection *connection, if (type_map_func) { errno = 0; typed_property_value = (*type_map_func) (property_value); - if(errno) + if (errno) goto wireless_next; } - g_object_set(wireless_security_setting, - newkey, typed_property_value ?: property_value, - NULL); + g_object_set (wireless_security_setting, + newkey, typed_property_value ?: property_value, + NULL); security = TRUE; - wireless_next: - g_free(property_value); +wireless_next: + g_free (property_value); if (typed_property_value && free_func) (*free_func) (typed_property_value); - } else if(strlen(curr->key) > wpa_l && - !strncmp("wpa-", curr->key, wpa_l)) { + } else if ( strlen (curr->key) > wpa_l + && !strncmp ("wpa-", curr->key, wpa_l)) { char *property_value = NULL; gpointer typed_property_value = NULL; - const char* newkey = map_by_mapping(mapping, curr->key+wpa_l); + const char* newkey = map_by_mapping (mapping, curr->key+wpa_l); IfupdownStrDupeFunc dupe_func = map_by_mapping (dupe_mapping, curr->key+wpa_l); IfupdownStrToTypeFunc type_map_func = map_by_mapping (type_mapping, curr->key+wpa_l); GFreeFunc free_func = map_by_mapping (free_type_mapping, curr->key+wpa_l); - if(!newkey || !dupe_func) + if (!newkey || !dupe_func) goto next; property_value = (*dupe_func) (curr->data, connection); @@ -365,14 +364,14 @@ update_wireless_security_setting_from_if_block(NMConnection *connection, #ifdef DEBUG_SECRETS property_value #else /* DEBUG_SECRETS */ - !strcmp("key", newkey) || - !strcmp("leap-password", newkey) || - !strcmp("pin", newkey) || - !strcmp("psk", newkey) || - !strcmp("wep-key0", newkey) || - !strcmp("wep-key1", newkey) || - !strcmp("wep-key2", newkey) || - !strcmp("wep-key3", newkey) || + !strcmp ("key", newkey) || + !strcmp ("leap-password", newkey) || + !strcmp ("pin", newkey) || + !strcmp ("psk", newkey) || + !strcmp ("wep-key0", newkey) || + !strcmp ("wep-key1", newkey) || + !strcmp ("wep-key2", newkey) || + !strcmp ("wep-key3", newkey) || NULL ? "" : property_value #endif /* DEBUG_SECRETS */ @@ -381,17 +380,17 @@ update_wireless_security_setting_from_if_block(NMConnection *connection, if (type_map_func) { errno = 0; typed_property_value = (*type_map_func) (property_value); - if(errno) + if (errno) goto wpa_next; } - g_object_set(wireless_security_setting, + g_object_set (wireless_security_setting, newkey, typed_property_value ?: property_value, NULL); security = TRUE; wpa_next: - g_free(property_value); + g_free (property_value); if (free_func && typed_property_value) (*free_func) (typed_property_value); } @@ -404,12 +403,12 @@ update_wireless_security_setting_from_if_block(NMConnection *connection, } static void -update_wired_setting_from_if_block(NMConnection *connection, - if_block *block) +update_wired_setting_from_if_block (NMConnection *connection, + if_block *block) { NMSettingWired *s_wired = NULL; - s_wired = NM_SETTING_WIRED(nm_setting_wired_new()); - nm_connection_add_setting(connection, NM_SETTING(s_wired)); + s_wired = NM_SETTING_WIRED (nm_setting_wired_new ()); + nm_connection_add_setting (connection, NM_SETTING (s_wired)); } static void @@ -438,14 +437,14 @@ ifupdown_ip4_add_dns (NMSettingIPConfig *s_ip4, const char *dns) } static gboolean -update_ip4_setting_from_if_block(NMConnection *connection, - if_block *block, - GError **error) +update_ip4_setting_from_if_block (NMConnection *connection, + if_block *block, + GError **error) { - NMSettingIPConfig *s_ip4 = NM_SETTING_IP_CONFIG (nm_setting_ip4_config_new()); - const char *type = ifparser_getkey(block, "inet"); - gboolean is_static = type && !strcmp("static", type); + NMSettingIPConfig *s_ip4 = NM_SETTING_IP_CONFIG (nm_setting_ip4_config_new ()); + const char *type = ifparser_getkey (block, "inet"); + gboolean is_static = type && !strcmp ("static", type); if (!is_static) { g_object_set (s_ip4, NM_SETTING_IP_CONFIG_METHOD, NM_SETTING_IP4_CONFIG_METHOD_AUTO, NULL); @@ -568,17 +567,18 @@ ifupdown_ip6_add_dns (NMSettingIPConfig *s_ip6, const char *dns) } static gboolean -update_ip6_setting_from_if_block(NMConnection *connection, - if_block *block, - GError **error) +update_ip6_setting_from_if_block (NMConnection *connection, + if_block *block, + GError **error) { - NMSettingIPConfig *s_ip6 = NM_SETTING_IP_CONFIG (nm_setting_ip6_config_new()); - const char *type = ifparser_getkey(block, "inet6"); - gboolean is_static = type && (!strcmp("static", type) || - !strcmp("v4tunnel", type)); + NMSettingIPConfig *s_ip6 = NM_SETTING_IP_CONFIG (nm_setting_ip6_config_new ()); + const char *type = ifparser_getkey (block, "inet6"); + gboolean is_static = type + && ( !strcmp ("static", type) + || !strcmp ("v4tunnel", type)); if (!is_static) { - g_object_set(s_ip6, NM_SETTING_IP_CONFIG_METHOD, NM_SETTING_IP6_CONFIG_METHOD_AUTO, NULL); + g_object_set (s_ip6, NM_SETTING_IP_CONFIG_METHOD, NM_SETTING_IP6_CONFIG_METHOD_AUTO, NULL); } else { NMIPAddress *addr; const char *address_v; @@ -591,7 +591,7 @@ update_ip6_setting_from_if_block(NMConnection *connection, char **list, **iter; /* Address */ - address_v = ifparser_getkey(block, "address"); + address_v = ifparser_getkey (block, "address"); if (!address_v) { g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION, "Missing IPv6 address"); @@ -599,7 +599,7 @@ update_ip6_setting_from_if_block(NMConnection *connection, } /* Prefix */ - prefix_v = ifparser_getkey(block, "netmask"); + prefix_v = ifparser_getkey (block, "netmask"); if (prefix_v) prefix_int = g_ascii_strtoll (prefix_v, NULL, 10); @@ -628,10 +628,10 @@ update_ip6_setting_from_if_block(NMConnection *connection, g_object_set (s_ip6, NM_SETTING_IP_CONFIG_GATEWAY, gateway_v, NULL); } - nameserver_v = ifparser_getkey(block, "dns-nameserver"); + nameserver_v = ifparser_getkey (block, "dns-nameserver"); ifupdown_ip6_add_dns (s_ip6, nameserver_v); - nameservers_v = ifparser_getkey(block, "dns-nameservers"); + nameservers_v = ifparser_getkey (block, "dns-nameservers"); ifupdown_ip6_add_dns (s_ip6, nameservers_v); if (!nm_setting_ip_config_get_num_dns (s_ip6)) @@ -677,8 +677,8 @@ ifupdown_update_connection_from_if_block (NMConnection *connection, gboolean success = FALSE; s_con = nm_connection_get_setting_connection (connection); - if(!s_con) { - s_con = NM_SETTING_CONNECTION (nm_setting_connection_new()); + if (!s_con) { + s_con = NM_SETTING_CONNECTION (nm_setting_connection_new ()); g_assert (s_con); nm_connection_add_setting (connection, NM_SETTING (s_con)); } @@ -708,7 +708,7 @@ ifupdown_update_connection_from_if_block (NMConnection *connection, update_wireless_security_setting_from_if_block (connection, block); } - if (ifparser_haskey(block, "inet6")) + if (ifparser_haskey (block, "inet6")) success = update_ip6_setting_from_if_block (connection, block, error); else success = update_ip4_setting_from_if_block (connection, block, error); -- cgit v1.2.1 From de2e75e3275a7b38703f3f74470953cc9e6f3a8c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 25 Aug 2018 15:16:52 +0200 Subject: settings/ifupdown: use nm_streq() in parser --- .../ifupdown/nms-ifupdown-interface-parser.c | 21 ++++--- .../plugins/ifupdown/nms-ifupdown-parser.c | 71 +++++++++++----------- 2 files changed, 45 insertions(+), 47 deletions(-) diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c b/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c index 178acb3039..df76775eee 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c @@ -190,7 +190,7 @@ _recursive_ifparser (const char *eni_file, int quiet) * Create a block for each of them except source and source-directory. */ /* iface stanza takes at least 3 parameters */ - if (strcmp (token[0], "iface") == 0) { + if (nm_streq (token[0], "iface")) { if (toknum < 4) { if (!quiet) { nm_log_warn (LOGD_SETTINGS, "Can't parse iface line '%s'\n", @@ -204,33 +204,33 @@ _recursive_ifparser (const char *eni_file, int quiet) } /* auto and allow-auto stanzas are equivalent, * both can take multiple interfaces as parameters: add one block for each */ - else if (strcmp (token[0], "auto") == 0 || - strcmp (token[0], "allow-auto") == 0) { + else if (NM_IN_STRSET (token[0], "auto", "allow-auto")) { int i; + for (i = 1; i < toknum; i++) add_block ("auto", token[i]); skip_to_block = 0; } - else if (strcmp (token[0], "mapping") == 0) { + else if (nm_streq (token[0], "mapping")) { add_block (token[0], join_values_with_spaces (value, token + 1)); skip_to_block = 0; } /* allow-* can take multiple interfaces as parameters: add one block for each */ - else if (strncmp (token[0],"allow-",6) == 0) { + else if (g_str_has_prefix (token[0], "allow-")) { int i; for (i = 1; i < toknum; i++) add_block (token[0], token[i]); skip_to_block = 0; } /* source and source-directory stanzas take one or more paths as parameters */ - else if (strcmp (token[0], "source") == 0 || strcmp (token[0], "source-directory") == 0) { + else if (NM_IN_STRSET (token[0], "source", "source-directory")) { int i; char *en_dir; skip_to_block = 0; en_dir = g_path_get_dirname (eni_file); for (i = 1; i < toknum; ++i) { - if (strcmp (token[0], "source-directory") == 0) + if (nm_streq (token[0], "source-directory")) _ifparser_source (token[i], en_dir, quiet, TRUE); else _ifparser_source (token[i], en_dir, quiet, FALSE); @@ -359,7 +359,8 @@ ifparser_getif (const char* iface) { if_block *curr = first; while (curr != NULL) { - if (strcmp (curr->type,"iface")==0 && strcmp (curr->name,iface)==0) + if ( nm_streq (curr->type, "iface") + && nm_streq (curr->name, iface)) return curr; curr = curr->next; } @@ -371,7 +372,7 @@ ifparser_getkey (if_block* iface, const char *key) { if_data *curr = iface->info; while (curr != NULL) { - if (strcmp (curr->key,key)==0) + if (nm_streq (curr->key, key)) return curr->data; curr = curr->next; } @@ -384,7 +385,7 @@ ifparser_haskey (if_block* iface, const char *key) if_data *curr = iface->info; while (curr != NULL) { - if (strcmp (curr->key, key) == 0) + if (nm_streq (curr->key, key)) return TRUE; curr = curr->next; } diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-parser.c b/src/settings/plugins/ifupdown/nms-ifupdown-parser.c index 4dd922b4a2..e5d1f90082 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-parser.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-parser.c @@ -43,13 +43,13 @@ _ifupdownplugin_guess_connection_type (if_block *block) if_data *curr = block->info; const char* ret_type = NULL; const char* value = ifparser_getkey (block, "inet"); - if (value && !strcmp ("ppp", value)) { + + if (nm_streq0 (value, "ppp")) ret_type = NM_SETTING_PPP_SETTING_NAME; - } while (!ret_type && curr) { if (!strncmp ("wireless-", curr->key, strlen ("wireless-")) || - !strncmp ("wpa-", curr->key, strlen ("wpa-"))) { + !strncmp ("wpa-", curr->key, strlen ("wpa-"))) { ret_type = NM_SETTING_WIRELESS_SETTING_NAME; } curr = curr->next; @@ -71,8 +71,9 @@ static gpointer map_by_mapping (struct _Mapping *mapping, const char *key) { struct _Mapping *curr = mapping; + while (curr->domain) { - if (!strcmp (curr->domain, key)) + if (nm_streq (curr->domain, key)) return curr->target; curr++; } @@ -97,9 +98,8 @@ update_wireless_setting_from_if_block (NMConnection *connection, NMSettingWireless *wireless_setting = NULL; - if (value && !strcmp ("ppp", value)) { + if (nm_streq0 (value, "ppp")) return; - } nm_log_info (LOGD_SETTINGS, "update wireless settings (%s).", block->name); wireless_setting = NM_SETTING_WIRELESS (nm_setting_wireless_new ()); @@ -109,7 +109,7 @@ update_wireless_setting_from_if_block (NMConnection *connection, !strncmp ("wireless-", curr->key, wireless_l)) { const char* newkey = map_by_mapping (mapping, curr->key+wireless_l); nm_log_info (LOGD_SETTINGS, "wireless setting key: %s='%s'", newkey, curr->data); - if (newkey && !strcmp ("ssid", newkey)) { + if (nm_streq0 (newkey, "ssid")) { GBytes *ssid; int len = strlen (curr->data); @@ -117,7 +117,7 @@ update_wireless_setting_from_if_block (NMConnection *connection, g_object_set (wireless_setting, NM_SETTING_WIRELESS_SSID, ssid, NULL); g_bytes_unref (ssid); nm_log_info (LOGD_SETTINGS, "setting wireless ssid = %d", len); - } else if (newkey && !strcmp ("mode", newkey)) { + } else if (nm_streq0 (newkey, "mode")) { if (!g_ascii_strcasecmp (curr->data, "Managed") || !g_ascii_strcasecmp (curr->data, "Auto")) g_object_set (wireless_setting, NM_SETTING_WIRELESS_MODE, NM_SETTING_WIRELESS_MODE_INFRA, NULL); else if (!g_ascii_strcasecmp (curr->data, "Ad-Hoc")) @@ -135,7 +135,7 @@ update_wireless_setting_from_if_block (NMConnection *connection, && !strncmp ("wpa-", curr->key, wpa_l)) { const char* newkey = map_by_mapping (mapping, curr->key+wpa_l); - if (newkey && !strcmp ("ssid", newkey)) { + if (nm_streq0 (newkey, "ssid")) { GBytes *ssid; int len = strlen (curr->data); @@ -302,9 +302,8 @@ update_wireless_security_setting_from_if_block (NMConnection *connection, NMSettingWireless *s_wireless; gboolean security = FALSE; - if (value && !strcmp ("ppp", value)) { + if (nm_streq0 (value, "ppp")) return; - } s_wireless = nm_connection_get_setting_wireless (connection); g_return_if_fail (s_wireless); @@ -361,20 +360,16 @@ wireless_next: property_value = (*dupe_func) (curr->data, connection); nm_log_info (LOGD_SETTINGS, "setting wpa security key: %s=%s", newkey, -#ifdef DEBUG_SECRETS - property_value -#else /* DEBUG_SECRETS */ - !strcmp ("key", newkey) || - !strcmp ("leap-password", newkey) || - !strcmp ("pin", newkey) || - !strcmp ("psk", newkey) || - !strcmp ("wep-key0", newkey) || - !strcmp ("wep-key1", newkey) || - !strcmp ("wep-key2", newkey) || - !strcmp ("wep-key3", newkey) || - NULL ? - "" : property_value -#endif /* DEBUG_SECRETS */ + NM_IN_STRSET (newkey, "key", + "leap-password", + "pin", + "psk", + "wep-key0", + "wep-key1", + "wep-key2", + "wep-key3") + ? "" + : property_value ); if (type_map_func) { @@ -385,8 +380,8 @@ wireless_next: } g_object_set (wireless_security_setting, - newkey, typed_property_value ?: property_value, - NULL); + newkey, typed_property_value ?: property_value, + NULL); security = TRUE; wpa_next: @@ -444,10 +439,12 @@ update_ip4_setting_from_if_block (NMConnection *connection, NMSettingIPConfig *s_ip4 = NM_SETTING_IP_CONFIG (nm_setting_ip4_config_new ()); const char *type = ifparser_getkey (block, "inet"); - gboolean is_static = type && !strcmp ("static", type); - if (!is_static) { - g_object_set (s_ip4, NM_SETTING_IP_CONFIG_METHOD, NM_SETTING_IP4_CONFIG_METHOD_AUTO, NULL); + if (!nm_streq0 (type, "static")) { + g_object_set (s_ip4, + NM_SETTING_IP_CONFIG_METHOD, + NM_SETTING_IP4_CONFIG_METHOD_AUTO, + NULL); } else { guint32 tmp_mask; NMIPAddress *addr; @@ -573,12 +570,12 @@ update_ip6_setting_from_if_block (NMConnection *connection, { NMSettingIPConfig *s_ip6 = NM_SETTING_IP_CONFIG (nm_setting_ip6_config_new ()); const char *type = ifparser_getkey (block, "inet6"); - gboolean is_static = type - && ( !strcmp ("static", type) - || !strcmp ("v4tunnel", type)); - if (!is_static) { - g_object_set (s_ip6, NM_SETTING_IP_CONFIG_METHOD, NM_SETTING_IP6_CONFIG_METHOD_AUTO, NULL); + if (!NM_IN_STRSET (type, "static", "v4tunnel")) { + g_object_set (s_ip6, + NM_SETTING_IP_CONFIG_METHOD, + NM_SETTING_IP6_CONFIG_METHOD_AUTO, + NULL); } else { NMIPAddress *addr; const char *address_v; @@ -701,9 +698,9 @@ ifupdown_update_connection_from_if_block (NMConnection *connection, nm_log_info (LOGD_SETTINGS, "update_connection_setting_from_if_block: name:%s, type:%s, id:%s, uuid: %s", block->name, type, idstr, nm_setting_connection_get_uuid (s_con)); - if (!strcmp (NM_SETTING_WIRED_SETTING_NAME, type)) + if (nm_streq (type, NM_SETTING_WIRED_SETTING_NAME)) update_wired_setting_from_if_block (connection, block); - else if (!strcmp (NM_SETTING_WIRELESS_SETTING_NAME, type)) { + else if (nm_streq (type, NM_SETTING_WIRELESS_SETTING_NAME)) { update_wireless_setting_from_if_block (connection, block); update_wireless_security_setting_from_if_block (connection, block); } -- cgit v1.2.1 From b8da0855fa564429bbc3058d999b62496694d4eb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 25 Aug 2018 20:46:14 +0200 Subject: settings/ifupdown: refactor string prefix matching in parser --- .../plugins/ifupdown/nms-ifupdown-parser.c | 55 +++++++++++----------- 1 file changed, 28 insertions(+), 27 deletions(-) diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-parser.c b/src/settings/plugins/ifupdown/nms-ifupdown-parser.c index e5d1f90082..c86cf7520f 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-parser.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-parser.c @@ -37,6 +37,15 @@ #include "nms-ifupdown-plugin.h" #include "nms-ifupdown-parser.h" +#define _str_has_prefix(val, prefix, require_suffix) \ + ({ \ + const char *_val = (val); \ + \ + (strncmp (_val, ""prefix"", NM_STRLEN (prefix)) == 0) \ + && ( !(require_suffix) \ + || _val[NM_STRLEN (prefix)] != '\0'); \ + }) + static const char* _ifupdownplugin_guess_connection_type (if_block *block) { @@ -48,8 +57,8 @@ _ifupdownplugin_guess_connection_type (if_block *block) ret_type = NM_SETTING_PPP_SETTING_NAME; while (!ret_type && curr) { - if (!strncmp ("wireless-", curr->key, strlen ("wireless-")) || - !strncmp ("wpa-", curr->key, strlen ("wpa-"))) { + if ( _str_has_prefix (curr->key, "wireless-", FALSE) + || _str_has_prefix (curr->key, "wpa-", FALSE)) { ret_type = NM_SETTING_WIRELESS_SETTING_NAME; } curr = curr->next; @@ -84,9 +93,6 @@ static void update_wireless_setting_from_if_block (NMConnection *connection, if_block *block) { - int wpa_l= strlen ("wpa-"); - int wireless_l= strlen ("wireless-"); - if_data *curr = block->info; const char* value = ifparser_getkey (block, "inet"); struct _Mapping mapping[] = { @@ -105,9 +111,9 @@ update_wireless_setting_from_if_block (NMConnection *connection, wireless_setting = NM_SETTING_WIRELESS (nm_setting_wireless_new ()); while (curr) { - if (strlen (curr->key) > wireless_l && - !strncmp ("wireless-", curr->key, wireless_l)) { - const char* newkey = map_by_mapping (mapping, curr->key+wireless_l); + if (_str_has_prefix (curr->key, "wireless-", TRUE)) { + const char* newkey = map_by_mapping (mapping, curr->key + NM_STRLEN ("wireless-")); + nm_log_info (LOGD_SETTINGS, "wireless setting key: %s='%s'", newkey, curr->data); if (nm_streq0 (newkey, "ssid")) { GBytes *ssid; @@ -131,9 +137,8 @@ update_wireless_setting_from_if_block (NMConnection *connection, newkey, curr->data, NULL); } - } else if ( strlen (curr->key) > wpa_l - && !strncmp ("wpa-", curr->key, wpa_l)) { - const char* newkey = map_by_mapping (mapping, curr->key+wpa_l); + } else if (_str_has_prefix (curr->key, "wpa-", TRUE)) { + const char* newkey = map_by_mapping (mapping, curr->key + NM_STRLEN ("wpa-")); if (nm_streq0 (newkey, "ssid")) { GBytes *ssid; @@ -243,8 +248,6 @@ static void update_wireless_security_setting_from_if_block (NMConnection *connection, if_block *block) { - int wpa_l= strlen ("wpa-"); - int wireless_l= strlen ("wireless-"); if_data *curr = block->info; const char* value = ifparser_getkey (block, "inet"); struct _Mapping mapping[] = { @@ -312,15 +315,14 @@ update_wireless_security_setting_from_if_block (NMConnection *connection, wireless_security_setting = NM_SETTING_WIRELESS_SECURITY (nm_setting_wireless_security_new ()); while (curr) { - if ( strlen (curr->key) > wireless_l - && !strncmp ("wireless-", curr->key, wireless_l)) { - + if (_str_has_prefix (curr->key, "wireless-", TRUE)) { + const char *key = curr->key + NM_STRLEN ("wireless-"); char *property_value = NULL; gpointer typed_property_value = NULL; - const char* newkey = map_by_mapping (mapping, curr->key+wireless_l); - IfupdownStrDupeFunc dupe_func = map_by_mapping (dupe_mapping, curr->key+wireless_l); - IfupdownStrToTypeFunc type_map_func = map_by_mapping (type_mapping, curr->key+wireless_l); - GFreeFunc free_func = map_by_mapping (free_type_mapping, curr->key+wireless_l); + const char* newkey = map_by_mapping (mapping, key); + IfupdownStrDupeFunc dupe_func = map_by_mapping (dupe_mapping, key); + IfupdownStrToTypeFunc type_map_func = map_by_mapping (type_mapping, key); + GFreeFunc free_func = map_by_mapping (free_type_mapping, key); if (!newkey || !dupe_func) goto next; @@ -345,15 +347,14 @@ wireless_next: if (typed_property_value && free_func) (*free_func) (typed_property_value); - } else if ( strlen (curr->key) > wpa_l - && !strncmp ("wpa-", curr->key, wpa_l)) { - + } else if (_str_has_prefix (curr->key, "wpa-", TRUE)) { + const char *key = curr->key + NM_STRLEN ("wpa-"); char *property_value = NULL; gpointer typed_property_value = NULL; - const char* newkey = map_by_mapping (mapping, curr->key+wpa_l); - IfupdownStrDupeFunc dupe_func = map_by_mapping (dupe_mapping, curr->key+wpa_l); - IfupdownStrToTypeFunc type_map_func = map_by_mapping (type_mapping, curr->key+wpa_l); - GFreeFunc free_func = map_by_mapping (free_type_mapping, curr->key+wpa_l); + const char* newkey = map_by_mapping (mapping, key); + IfupdownStrDupeFunc dupe_func = map_by_mapping (dupe_mapping, key); + IfupdownStrToTypeFunc type_map_func = map_by_mapping (type_mapping, key); + GFreeFunc free_func = map_by_mapping (free_type_mapping, key); if (!newkey || !dupe_func) goto next; -- cgit v1.2.1 From 6f14228cb388b9fa2786b2baa62caefee0f355e5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 25 Aug 2018 13:19:31 +0200 Subject: settings/ifupdown: hide internal functions in "nms-ifupdown-interface-parser.h" --- src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c | 8 ++++---- src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.h | 5 ----- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c b/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c index df76775eee..b1d6e4a105 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c @@ -36,7 +36,7 @@ if_block* first; if_block* last; if_data* last_data; -void +static void add_block (const char *type, const char* name) { if_block *ret = g_slice_new0 (struct _if_block); @@ -51,7 +51,7 @@ add_block (const char *type, const char* name) last_data = NULL; } -void +static void add_data (const char *key, const char *data) { if_data *ret; @@ -305,7 +305,7 @@ ifparser_init (const char *eni_file, int quiet) _recursive_ifparser (eni_file, quiet); } -void +static void _destroy_data (if_data *ifd) { if (ifd == NULL) @@ -317,7 +317,7 @@ _destroy_data (if_data *ifd) return; } -void +static void _destroy_block (if_block* ifb) { if (ifb == NULL) diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.h b/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.h index 62b658363f..68c92944b1 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.h +++ b/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.h @@ -46,9 +46,4 @@ gboolean ifparser_haskey (if_block* iface, const char *key); int ifparser_get_num_blocks (void); int ifparser_get_num_info (if_block* iface); -void add_block (const char *type, const char* name); -void add_data (const char *key,const char *data); -void _destroy_data (if_data *ifd); -void _destroy_block (if_block* ifb); - #endif -- cgit v1.2.1 From 70350bb6212a17135222f09aec479750b1a3bf82 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 25 Aug 2018 13:26:46 +0200 Subject: settings/ifupdown: don't use global variables for /e/n/i parser --- .../ifupdown/nms-ifupdown-interface-parser.c | 95 +++++---- .../ifupdown/nms-ifupdown-interface-parser.h | 15 +- .../plugins/ifupdown/nms-ifupdown-plugin.c | 7 +- .../plugins/ifupdown/tests/test-ifupdown.c | 231 +++++++++------------ 4 files changed, 163 insertions(+), 185 deletions(-) diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c b/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c index b1d6e4a105..84650bb6a6 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c @@ -32,33 +32,41 @@ #include "nm-utils.h" -if_block* first; -if_block* last; -if_data* last_data; +struct _if_parser { + if_block* first; + if_block* last; + if_data* last_data; +}; + +/*****************************************************************************/ + +static void _ifparser_source (if_parser *parser, const char *path, const char *en_dir, int quiet, int dir); + +/*****************************************************************************/ static void -add_block (const char *type, const char* name) +add_block (if_parser *parser, const char *type, const char* name) { if_block *ret = g_slice_new0 (struct _if_block); ret->name = g_strdup (name); ret->type = g_strdup (type); - if (first == NULL) - first = last = ret; + if (parser->first == NULL) + parser->first = parser->last = ret; else { - last->next = ret; - last = ret; + parser->last->next = ret; + parser->last = ret; } - last_data = NULL; + parser->last_data = NULL; } static void -add_data (const char *key, const char *data) +add_data (if_parser *parser, const char *key, const char *data) { if_data *ret; char *idx; /* Check if there is a block where we can attach our data */ - if (first == NULL) + if (parser->first == NULL) return; ret = g_slice_new0 (struct _if_data); @@ -71,12 +79,12 @@ add_data (const char *key, const char *data) } ret->data = g_strdup (data); - if (last->info == NULL) { - last->info = ret; - last_data = ret; + if (parser->last->info == NULL) { + parser->last->info = ret; + parser->last_data = ret; } else { - last_data->next = ret; - last_data = last_data->next; + parser->last_data->next = ret; + parser->last_data = parser->last_data->next; } } @@ -98,10 +106,8 @@ join_values_with_spaces (char *dst, char **src) return (dst); } -static void _ifparser_source (const char *path, const char *en_dir, int quiet, int dir); - static void -_recursive_ifparser (const char *eni_file, int quiet) +_recursive_ifparser (if_parser *parser, const char *eni_file, int quiet) { FILE *inp; char line[255]; @@ -198,9 +204,9 @@ _recursive_ifparser (const char *eni_file, int quiet) } continue; } - add_block (token[0], token[1]); + add_block (parser, token[0], token[1]); skip_to_block = 0; - add_data (token[2], join_values_with_spaces (value, token + 3)); + add_data (parser, token[2], join_values_with_spaces (value, token + 3)); } /* auto and allow-auto stanzas are equivalent, * both can take multiple interfaces as parameters: add one block for each */ @@ -208,18 +214,18 @@ _recursive_ifparser (const char *eni_file, int quiet) int i; for (i = 1; i < toknum; i++) - add_block ("auto", token[i]); + add_block (parser, "auto", token[i]); skip_to_block = 0; } else if (nm_streq (token[0], "mapping")) { - add_block (token[0], join_values_with_spaces (value, token + 1)); + add_block (parser, token[0], join_values_with_spaces (value, token + 1)); skip_to_block = 0; } /* allow-* can take multiple interfaces as parameters: add one block for each */ else if (g_str_has_prefix (token[0], "allow-")) { int i; for (i = 1; i < toknum; i++) - add_block (token[0], token[i]); + add_block (parser, token[0], token[i]); skip_to_block = 0; } /* source and source-directory stanzas take one or more paths as parameters */ @@ -231,9 +237,9 @@ _recursive_ifparser (const char *eni_file, int quiet) en_dir = g_path_get_dirname (eni_file); for (i = 1; i < toknum; ++i) { if (nm_streq (token[0], "source-directory")) - _ifparser_source (token[i], en_dir, quiet, TRUE); + _ifparser_source (parser, token[i], en_dir, quiet, TRUE); else - _ifparser_source (token[i], en_dir, quiet, FALSE); + _ifparser_source (parser, token[i], en_dir, quiet, FALSE); } g_free (en_dir); } @@ -244,7 +250,7 @@ _recursive_ifparser (const char *eni_file, int quiet) join_values_with_spaces (value, token)); } } else - add_data (token[0], join_values_with_spaces (value, token + 1)); + add_data (parser, token[0], join_values_with_spaces (value, token + 1)); } } fclose (inp); @@ -254,7 +260,7 @@ _recursive_ifparser (const char *eni_file, int quiet) } static void -_ifparser_source (const char *path, const char *en_dir, int quiet, int dir) +_ifparser_source (if_parser *parser, const char *path, const char *en_dir, int quiet, int dir) { char *abs_path; const char *item; @@ -287,22 +293,25 @@ _ifparser_source (const char *path, const char *en_dir, int quiet, int dir) g_clear_error (&error); } else { while ((item = g_dir_read_name (source_dir))) - _ifparser_source (item, we.we_wordv[i], quiet, FALSE); + _ifparser_source (parser, item, we.we_wordv[i], quiet, FALSE); g_dir_close (source_dir); } } else - _recursive_ifparser (we.we_wordv[i], quiet); + _recursive_ifparser (parser, we.we_wordv[i], quiet); } wordfree (&we); } g_free (abs_path); } -void -ifparser_init (const char *eni_file, int quiet) +if_parser * +ifparser_parse (const char *eni_file, int quiet) { - first = last = NULL; - _recursive_ifparser (eni_file, quiet); + if_parser *parser; + + parser = g_slice_new0 (if_parser); + _recursive_ifparser (parser, eni_file, quiet); + return parser; } static void @@ -331,21 +340,21 @@ _destroy_block (if_block* ifb) } void -ifparser_destroy (void) +ifparser_destroy (if_parser *parser) { - _destroy_block (first); - first = last = NULL; + _destroy_block (parser->first); + g_slice_free (if_parser, parser); } -if_block *ifparser_getfirst (void) +if_block *ifparser_getfirst (if_parser *parser) { - return first; + return parser->first; } -int ifparser_get_num_blocks (void) +int ifparser_get_num_blocks (if_parser *parser) { int i = 0; - if_block *iter = first; + if_block *iter = parser->first; while (iter) { i++; @@ -355,9 +364,9 @@ int ifparser_get_num_blocks (void) } if_block * -ifparser_getif (const char* iface) +ifparser_getif (if_parser *parser, const char* iface) { - if_block *curr = first; + if_block *curr = parser->first; while (curr != NULL) { if ( nm_streq (curr->type, "iface") && nm_streq (curr->name, iface)) diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.h b/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.h index 68c92944b1..d6f1871109 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.h +++ b/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.h @@ -36,14 +36,19 @@ typedef struct _if_block { struct _if_block *next; } if_block; -void ifparser_init (const char *eni_file, int quiet); -void ifparser_destroy (void); +typedef struct _if_parser if_parser; -if_block *ifparser_getif (const char* iface); -if_block *ifparser_getfirst (void); +if_parser *ifparser_parse (const char *eni_file, int quiet); + +void ifparser_destroy (if_parser *parser); +NM_AUTO_DEFINE_FCN0 (if_parser *, _nm_auto_ifparser, ifparser_destroy); +#define nm_auto_ifparser nm_auto(_nm_auto_ifparser) + +if_block *ifparser_getif (if_parser *parser, const char* iface); +if_block *ifparser_getfirst (if_parser *parser); const char *ifparser_getkey (if_block* iface, const char *key); gboolean ifparser_haskey (if_block* iface, const char *key); -int ifparser_get_num_blocks (void); +int ifparser_get_num_blocks (if_parser *parser); int ifparser_get_num_info (if_block* iface); #endif diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c index 07bf7ae985..2e86d3f521 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c @@ -152,14 +152,15 @@ initialize (NMSettingsPlugin *plugin) SettingsPluginIfupdown *self = SETTINGS_PLUGIN_IFUPDOWN (plugin); SettingsPluginIfupdownPrivate *priv = SETTINGS_PLUGIN_IFUPDOWN_GET_PRIVATE (self); gs_unref_hashtable GHashTable *auto_ifaces = NULL; - if_block *block = NULL; + nm_auto_ifparser if_parser *parser = NULL; + if_block *block; GHashTableIter con_iter; const char *block_name; NMIfupdownConnection *conn; /* Read in all the interfaces */ - ifparser_init (ENI_INTERFACES_FILE, 0); - for (block = ifparser_getfirst (); block; block = block->next) { + parser = ifparser_parse (ENI_INTERFACES_FILE, 0); + for (block = ifparser_getfirst (parser); block; block = block->next) { if (NM_IN_STRSET (block->type, "auto", "allow-hotplug")) { if (!auto_ifaces) diff --git a/src/settings/plugins/ifupdown/tests/test-ifupdown.c b/src/settings/plugins/ifupdown/tests/test-ifupdown.c index 18bad65e36..634ec9af29 100644 --- a/src/settings/plugins/ifupdown/tests/test-ifupdown.c +++ b/src/settings/plugins/ifupdown/tests/test-ifupdown.c @@ -130,14 +130,14 @@ expected_free (Expected *e) } static void -compare_expected_to_ifparser (Expected *e) +compare_expected_to_ifparser (if_parser *parser, Expected *e) { if_block *n; GSList *biter, *kiter; - g_assert_cmpint (g_slist_length (e->blocks), ==, ifparser_get_num_blocks ()); + g_assert_cmpint (g_slist_length (e->blocks), ==, ifparser_get_num_blocks (parser)); - for (n = ifparser_getfirst (), biter = e->blocks; + for (n = ifparser_getfirst (parser), biter = e->blocks; n && biter; n = n->next, biter = g_slist_next (biter)) { if_data *m; @@ -164,12 +164,12 @@ compare_expected_to_ifparser (Expected *e) } static void -dump_blocks (void) +dump_blocks (if_parser *parser) { if_block *n; g_message ("\n***************************************************"); - for (n = ifparser_getfirst (); n != NULL; n = n->next) { + for (n = ifparser_getfirst (parser); n != NULL; n = n->next) { if_data *m; // each block start with its type & name @@ -187,21 +187,24 @@ dump_blocks (void) g_message ("##################################################\n"); } -static void -init_ifparser_with_file (const char *path, const char *file) +static if_parser * +init_ifparser_with_file (const char *file) { - char *tmp; + if_parser *parser; + gs_free char *tmp = NULL; - tmp = g_strdup_printf ("%s/%s", path, file); - ifparser_init (tmp, 1); - g_free (tmp); + tmp = g_strdup_printf ("%s/%s", TEST_DIR, file); + parser = ifparser_parse (tmp, 1); + g_assert (parser); + return parser; } static void -test1_ignore_line_before_first_block (const char *path) +test1_ignore_line_before_first_block (void) { Expected *e; ExpectedBlock *b; + nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test1"); e = expected_new (); b = expected_block_new ("auto", "eth0"); @@ -210,35 +213,33 @@ test1_ignore_line_before_first_block (const char *path) expected_add_block (e, b); expected_block_add_key (b, expected_key_new ("inet", "dhcp")); - init_ifparser_with_file (path, "test1"); - compare_expected_to_ifparser (e); + compare_expected_to_ifparser (parser, e); - ifparser_destroy (); expected_free (e); } static void -test2_wrapped_line (const char *path) +test2_wrapped_line (void) { Expected *e; ExpectedBlock *b; + nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test2"); e = expected_new (); b = expected_block_new ("auto", "lo"); expected_add_block (e, b); - init_ifparser_with_file (path, "test2"); - compare_expected_to_ifparser (e); + compare_expected_to_ifparser (parser, e); - ifparser_destroy (); expected_free (e); } static void -test3_wrapped_multiline_multiarg (const char *path) +test3_wrapped_multiline_multiarg (void) { Expected *e; ExpectedBlock *b; + nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test3"); e = expected_new (); b = expected_block_new ("allow-hotplug", "eth0"); @@ -248,35 +249,33 @@ test3_wrapped_multiline_multiarg (const char *path) b = expected_block_new ("allow-hotplug", "bnep0"); expected_add_block (e, b); - init_ifparser_with_file (path, "test3"); - compare_expected_to_ifparser (e); + compare_expected_to_ifparser (parser, e); - ifparser_destroy (); expected_free (e); } static void -test4_allow_auto_is_auto (const char *path) +test4_allow_auto_is_auto (void) { Expected *e; ExpectedBlock *b; + nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test4"); e = expected_new (); b = expected_block_new ("auto", "eth0"); expected_add_block (e, b); - init_ifparser_with_file (path, "test4"); - compare_expected_to_ifparser (e); + compare_expected_to_ifparser (parser, e); - ifparser_destroy (); expected_free (e); } static void -test5_allow_auto_multiarg (const char *path) +test5_allow_auto_multiarg (void) { Expected *e; ExpectedBlock *b; + nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test5"); e = expected_new (); b = expected_block_new ("allow-hotplug", "eth0"); @@ -284,52 +283,50 @@ test5_allow_auto_multiarg (const char *path) b = expected_block_new ("allow-hotplug", "wlan0"); expected_add_block (e, b); - init_ifparser_with_file (path, "test5"); - compare_expected_to_ifparser (e); + compare_expected_to_ifparser (parser, e); - ifparser_destroy (); expected_free (e); } static void -test6_mixed_whitespace (const char *path) +test6_mixed_whitespace (void) { Expected *e; ExpectedBlock *b; + nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test6"); e = expected_new (); b = expected_block_new ("iface", "lo"); expected_block_add_key (b, expected_key_new ("inet", "loopback")); expected_add_block (e, b); - init_ifparser_with_file (path, "test6"); - compare_expected_to_ifparser (e); + compare_expected_to_ifparser (parser, e); - ifparser_destroy (); expected_free (e); } static void -test7_long_line (const char *path) +test7_long_line (void) { - init_ifparser_with_file (path, "test7"); - g_assert_cmpint (ifparser_get_num_blocks (), ==, 0); - ifparser_destroy (); + nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test7"); + + g_assert_cmpint (ifparser_get_num_blocks (parser), ==, 0); } static void -test8_long_line_wrapped (const char *path) +test8_long_line_wrapped (void) { - init_ifparser_with_file (path, "test8"); - g_assert_cmpint (ifparser_get_num_blocks (), ==, 0); - ifparser_destroy (); + nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test8"); + + g_assert_cmpint (ifparser_get_num_blocks (parser), ==, 0); } static void -test9_wrapped_lines_in_block (const char *path) +test9_wrapped_lines_in_block (void) { Expected *e; ExpectedBlock *b; + nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test9"); e = expected_new (); b = expected_block_new ("iface", "eth0"); @@ -340,18 +337,17 @@ test9_wrapped_lines_in_block (const char *path) expected_block_add_key (b, expected_key_new ("broadcast", "10.250.2.63")); expected_block_add_key (b, expected_key_new ("gateway", "10.250.2.50")); - init_ifparser_with_file (path, "test9"); - compare_expected_to_ifparser (e); + compare_expected_to_ifparser (parser, e); - ifparser_destroy (); expected_free (e); } static void -test11_complex_wrap (const char *path) +test11_complex_wrap (void) { Expected *e; ExpectedBlock *b; + nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test11"); e = expected_new (); b = expected_block_new ("iface", "pppoe"); @@ -359,18 +355,17 @@ test11_complex_wrap (const char *path) expected_block_add_key (b, expected_key_new ("inet", "manual")); expected_block_add_key (b, expected_key_new ("pre-up", "/sbin/ifconfig eth0 up")); - init_ifparser_with_file (path, "test11"); - compare_expected_to_ifparser (e); + compare_expected_to_ifparser (parser, e); - ifparser_destroy (); expected_free (e); } static void -test12_complex_wrap_split_word (const char *path) +test12_complex_wrap_split_word (void) { Expected *e; ExpectedBlock *b; + nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test12"); e = expected_new (); b = expected_block_new ("iface", "pppoe"); @@ -378,36 +373,34 @@ test12_complex_wrap_split_word (const char *path) expected_block_add_key (b, expected_key_new ("inet", "manual")); expected_block_add_key (b, expected_key_new ("up", "ifup ppp0=dsl")); - init_ifparser_with_file (path, "test12"); - compare_expected_to_ifparser (e); + compare_expected_to_ifparser (parser, e); - ifparser_destroy (); expected_free (e); } static void -test13_more_mixed_whitespace (const char *path) +test13_more_mixed_whitespace (void) { Expected *e; ExpectedBlock *b; + nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test13"); e = expected_new (); b = expected_block_new ("iface", "dsl"); expected_block_add_key (b, expected_key_new ("inet", "ppp")); expected_add_block (e, b); - init_ifparser_with_file (path, "test13"); - compare_expected_to_ifparser (e); + compare_expected_to_ifparser (parser, e); - ifparser_destroy (); expected_free (e); } static void -test14_mixed_whitespace_block_start (const char *path) +test14_mixed_whitespace_block_start (void) { Expected *e; ExpectedBlock *b; + nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test14"); e = expected_new (); b = expected_block_new ("iface", "wlan0"); @@ -420,47 +413,43 @@ test14_mixed_whitespace_block_start (const char *path) expected_block_add_key (b, expected_key_new ("inet", "dhcp")); expected_add_block (e, b); - init_ifparser_with_file (path, "test14"); - compare_expected_to_ifparser (e); + compare_expected_to_ifparser (parser, e); - ifparser_destroy (); expected_free (e); } static void -test15_trailing_space (const char *path) +test15_trailing_space (void) { Expected *e; ExpectedBlock *b; + nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test15"); e = expected_new (); b = expected_block_new ("iface", "bnep0"); expected_block_add_key (b, expected_key_new ("inet", "static")); expected_add_block (e, b); - init_ifparser_with_file (path, "test15"); - compare_expected_to_ifparser (e); + compare_expected_to_ifparser (parser, e); - ifparser_destroy (); expected_free (e); } static void -test16_missing_newline (const char *path) +test16_missing_newline (void) { Expected *e; + nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test16"); e = expected_new (); expected_add_block (e, expected_block_new ("mapping", "eth0")); - init_ifparser_with_file (path, "test16"); - compare_expected_to_ifparser (e); + compare_expected_to_ifparser (parser, e); - ifparser_destroy (); expected_free (e); } static void -test17_read_static_ipv4 (const char *path) +test17_read_static_ipv4 (void) { NMConnection *connection; NMSettingConnection *s_con; @@ -470,9 +459,9 @@ test17_read_static_ipv4 (const char *path) gboolean success; NMIPAddress *ip4_addr; if_block *block = NULL; + nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test17-wired-static-verify-ip4"); - init_ifparser_with_file (path, "test17-wired-static-verify-ip4"); - block = ifparser_getfirst (); + block = ifparser_getfirst (parser); connection = nm_simple_connection_new(); g_assert (connection); @@ -511,12 +500,11 @@ test17_read_static_ipv4 (const char *path) g_assert_cmpstr (nm_setting_ip_config_get_dns_search (s_ip4, 0), ==, "example.com"); g_assert_cmpstr (nm_setting_ip_config_get_dns_search (s_ip4, 1), ==, "foo.example.com"); - ifparser_destroy (); g_object_unref (connection); } static void -test18_read_static_ipv6 (const char *path) +test18_read_static_ipv6 (void) { NMConnection *connection; NMSettingConnection *s_con; @@ -526,9 +514,9 @@ test18_read_static_ipv6 (const char *path) gboolean success; NMIPAddress *ip6_addr; if_block *block = NULL; + nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test18-wired-static-verify-ip6"); - init_ifparser_with_file (path, "test18-wired-static-verify-ip6"); - block = ifparser_getfirst (); + block = ifparser_getfirst (parser); connection = nm_simple_connection_new(); g_assert (connection); ifupdown_update_connection_from_if_block(connection, block, &error); @@ -566,12 +554,11 @@ test18_read_static_ipv6 (const char *path) g_assert_cmpstr (nm_setting_ip_config_get_dns_search (s_ip6, 0), ==, "example.com"); g_assert_cmpstr (nm_setting_ip_config_get_dns_search (s_ip6, 1), ==, "foo.example.com"); - ifparser_destroy (); g_object_unref (connection); } static void -test19_read_static_ipv4_plen (const char *path) +test19_read_static_ipv4_plen (void) { NMConnection *connection; NMSettingIPConfig *s_ip4; @@ -579,9 +566,9 @@ test19_read_static_ipv4_plen (const char *path) NMIPAddress *ip4_addr; if_block *block = NULL; gboolean success; + nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test19-wired-static-verify-ip4-plen"); - init_ifparser_with_file (path, "test19-wired-static-verify-ip4-plen"); - block = ifparser_getfirst (); + block = ifparser_getfirst (parser); connection = nm_simple_connection_new(); g_assert (connection); ifupdown_update_connection_from_if_block(connection, block, &error); @@ -601,15 +588,15 @@ test19_read_static_ipv4_plen (const char *path) g_assert_cmpstr (nm_ip_address_get_address (ip4_addr), ==, "10.0.0.3"); g_assert_cmpint (nm_ip_address_get_prefix (ip4_addr), ==, 8); - ifparser_destroy (); g_object_unref (connection); } static void -test20_source_stanza (const char *path) +test20_source_stanza (void) { Expected *e; ExpectedBlock *b; + nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test20-source-stanza"); e = expected_new (); @@ -625,18 +612,17 @@ test20_source_stanza (const char *path) expected_add_block (e, b); expected_block_add_key (b, expected_key_new ("inet", "dhcp")); - init_ifparser_with_file (path, "test20-source-stanza"); - compare_expected_to_ifparser (e); + compare_expected_to_ifparser (parser, e); - ifparser_destroy (); expected_free (e); } static void -test21_source_dir_stanza (const char *path) +test21_source_dir_stanza (void) { Expected *e; ExpectedBlock *b; + nm_auto_ifparser if_parser *parser = init_ifparser_with_file ("test21-source-dir-stanza"); e = expected_new (); @@ -646,10 +632,8 @@ test21_source_dir_stanza (const char *path) expected_add_block (e, b); expected_block_add_key (b, expected_key_new ("inet", "dhcp")); - init_ifparser_with_file (path, "test21-source-dir-stanza"); - compare_expected_to_ifparser (e); + compare_expected_to_ifparser (parser, e); - ifparser_destroy (); expected_free (e); } @@ -660,49 +644,28 @@ main (int argc, char **argv) { nmtst_init_assert_logging (&argc, &argv, "WARN", "DEFAULT"); - if (0) - dump_blocks (); - - g_test_add_data_func ("/ifupdate/ignore_line_before_first_block", TEST_DIR, - (GTestDataFunc) test1_ignore_line_before_first_block); - g_test_add_data_func ("/ifupdate/wrapped_line", TEST_DIR, - (GTestDataFunc) test2_wrapped_line); - g_test_add_data_func ("/ifupdate/wrapped_multiline_multiarg", TEST_DIR, - (GTestDataFunc) test3_wrapped_multiline_multiarg); - g_test_add_data_func ("/ifupdate/allow_auto_is_auto", TEST_DIR, - (GTestDataFunc) test4_allow_auto_is_auto); - g_test_add_data_func ("/ifupdate/allow_auto_multiarg", TEST_DIR, - (GTestDataFunc) test5_allow_auto_multiarg); - g_test_add_data_func ("/ifupdate/mixed_whitespace", TEST_DIR, - (GTestDataFunc) test6_mixed_whitespace); - g_test_add_data_func ("/ifupdate/long_line", TEST_DIR, - (GTestDataFunc) test7_long_line); - g_test_add_data_func ("/ifupdate/long_line_wrapped", TEST_DIR, - (GTestDataFunc) test8_long_line_wrapped); - g_test_add_data_func ("/ifupdate/wrapped_lines_in_block", TEST_DIR, - (GTestDataFunc) test9_wrapped_lines_in_block); - g_test_add_data_func ("/ifupdate/complex_wrap", TEST_DIR, - (GTestDataFunc) test11_complex_wrap); - g_test_add_data_func ("/ifupdate/complex_wrap_split_word", TEST_DIR, - (GTestDataFunc) test12_complex_wrap_split_word); - g_test_add_data_func ("/ifupdate/more_mixed_whitespace", TEST_DIR, - (GTestDataFunc) test13_more_mixed_whitespace); - g_test_add_data_func ("/ifupdate/mixed_whitespace_block_start", TEST_DIR, - (GTestDataFunc) test14_mixed_whitespace_block_start); - g_test_add_data_func ("/ifupdate/trailing_space", TEST_DIR, - (GTestDataFunc) test15_trailing_space); - g_test_add_data_func ("/ifupdate/missing_newline", TEST_DIR, - (GTestDataFunc) test16_missing_newline); - g_test_add_data_func ("/ifupdate/read_static_ipv4", TEST_DIR, - (GTestDataFunc) test17_read_static_ipv4); - g_test_add_data_func ("/ifupdate/read_static_ipv6", TEST_DIR, - (GTestDataFunc) test18_read_static_ipv6); - g_test_add_data_func ("/ifupdate/read_static_ipv4_plen", TEST_DIR, - (GTestDataFunc) test19_read_static_ipv4_plen); - g_test_add_data_func ("/ifupdate/source_stanza", TEST_DIR, - (GTestDataFunc) test20_source_stanza); - g_test_add_data_func ("/ifupdate/source_dir_stanza", TEST_DIR, - (GTestDataFunc) test21_source_dir_stanza); + (void) dump_blocks; + + g_test_add_func ("/ifupdate/ignore_line_before_first_block", test1_ignore_line_before_first_block); + g_test_add_func ("/ifupdate/wrapped_line", test2_wrapped_line); + g_test_add_func ("/ifupdate/wrapped_multiline_multiarg", test3_wrapped_multiline_multiarg); + g_test_add_func ("/ifupdate/allow_auto_is_auto", test4_allow_auto_is_auto); + g_test_add_func ("/ifupdate/allow_auto_multiarg", test5_allow_auto_multiarg); + g_test_add_func ("/ifupdate/mixed_whitespace", test6_mixed_whitespace); + g_test_add_func ("/ifupdate/long_line", test7_long_line); + g_test_add_func ("/ifupdate/long_line_wrapped", test8_long_line_wrapped); + g_test_add_func ("/ifupdate/wrapped_lines_in_block", test9_wrapped_lines_in_block); + g_test_add_func ("/ifupdate/complex_wrap", test11_complex_wrap); + g_test_add_func ("/ifupdate/complex_wrap_split_word", test12_complex_wrap_split_word); + g_test_add_func ("/ifupdate/more_mixed_whitespace", test13_more_mixed_whitespace); + g_test_add_func ("/ifupdate/mixed_whitespace_block_start", test14_mixed_whitespace_block_start); + g_test_add_func ("/ifupdate/trailing_space", test15_trailing_space); + g_test_add_func ("/ifupdate/missing_newline", test16_missing_newline); + g_test_add_func ("/ifupdate/read_static_ipv4", test17_read_static_ipv4); + g_test_add_func ("/ifupdate/read_static_ipv6", test18_read_static_ipv6); + g_test_add_func ("/ifupdate/read_static_ipv4_plen", test19_read_static_ipv4_plen); + g_test_add_func ("/ifupdate/source_stanza", test20_source_stanza); + g_test_add_func ("/ifupdate/source_dir_stanza", test21_source_dir_stanza); return g_test_run (); } -- cgit v1.2.1 From fe018a7e815c5adad0cfadc603662678732f759b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 25 Aug 2018 14:08:20 +0200 Subject: settings/ifupdown: use c-list for data structure of ifupdown parser We already have a linked-list implementation. Use it. --- .../ifupdown/nms-ifupdown-interface-parser.c | 149 +++++++++------------ .../ifupdown/nms-ifupdown-interface-parser.h | 21 +-- .../plugins/ifupdown/nms-ifupdown-parser.c | 39 +++--- .../plugins/ifupdown/nms-ifupdown-plugin.c | 4 +- .../plugins/ifupdown/tests/test-ifupdown.c | 28 ++-- 5 files changed, 114 insertions(+), 127 deletions(-) diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c b/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c index 84650bb6a6..b833dacbd7 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c @@ -32,12 +32,6 @@ #include "nm-utils.h" -struct _if_parser { - if_block* first; - if_block* last; - if_data* last_data; -}; - /*****************************************************************************/ static void _ifparser_source (if_parser *parser, const char *path, const char *en_dir, int quiet, int dir); @@ -47,45 +41,39 @@ static void _ifparser_source (if_parser *parser, const char *path, const char *e static void add_block (if_parser *parser, const char *type, const char* name) { - if_block *ret = g_slice_new0 (struct _if_block); - ret->name = g_strdup (name); - ret->type = g_strdup (type); - if (parser->first == NULL) - parser->first = parser->last = ret; - else { - parser->last->next = ret; - parser->last = ret; - } - parser->last_data = NULL; + if_block *ifb; + + ifb = g_slice_new (if_block); + ifb->name = g_strdup (name); + ifb->type = g_strdup (type); + c_list_init (&ifb->data_lst_head); + c_list_link_tail (&parser->block_lst_head, &ifb->block_lst); } static void add_data (if_parser *parser, const char *key, const char *data) { - if_data *ret; + if_block *last_block; + if_data *ifd; char *idx; + last_block = c_list_last_entry (&parser->block_lst_head, if_block, block_lst); + /* Check if there is a block where we can attach our data */ - if (parser->first == NULL) + if (!last_block) return; - ret = g_slice_new0 (struct _if_data); - ret->key = g_strdup (key); + ifd = g_slice_new (if_data); + ifd->key = g_strdup (key); + ifd->data = g_strdup (data); /* Normalize keys. Convert '_' to '-', as ifupdown accepts both variants. * When querying keys via ifparser_getkey(), use '-'. */ - while ((idx = strrchr (ret->key, '_'))) { - *idx = '-'; - } - ret->data = g_strdup (data); + idx = ifd->key; + while ((idx = strchr (idx, '_'))) + *(idx++) = '-'; - if (parser->last->info == NULL) { - parser->last->info = ret; - parser->last_data = ret; - } else { - parser->last_data->next = ret; - parser->last_data = parser->last_data->next; - } + c_list_link_tail (&last_block->data_lst_head, &ifd->data_lst); } /* join values in src with spaces into dst; dst needs to be large enough */ @@ -309,7 +297,8 @@ ifparser_parse (const char *eni_file, int quiet) { if_parser *parser; - parser = g_slice_new0 (if_parser); + parser = g_slice_new (if_parser); + c_list_init (&parser->block_lst_head); _recursive_ifparser (parser, eni_file, quiet); return parser; } @@ -317,99 +306,89 @@ ifparser_parse (const char *eni_file, int quiet) static void _destroy_data (if_data *ifd) { - if (ifd == NULL) - return; - _destroy_data (ifd->next); + c_list_unlink_stale (&ifd->data_lst); g_free (ifd->key); g_free (ifd->data); - g_slice_free (struct _if_data, ifd); - return; + g_slice_free (if_data, ifd); } static void _destroy_block (if_block* ifb) { - if (ifb == NULL) - return; - _destroy_block (ifb->next); - _destroy_data (ifb->info); + if_data *ifd; + + while ((ifd = c_list_first_entry (&ifb->data_lst_head, if_data, data_lst))) + _destroy_data (ifd); + c_list_unlink_stale (&ifb->block_lst); g_free (ifb->name); g_free (ifb->type); - g_slice_free (struct _if_block, ifb); - return; + g_slice_free (if_block, ifb); } void ifparser_destroy (if_parser *parser) { - _destroy_block (parser->first); + if_block *ifb; + + while ((ifb = c_list_first_entry (&parser->block_lst_head, if_block, block_lst))) + _destroy_block (ifb); g_slice_free (if_parser, parser); } -if_block *ifparser_getfirst (if_parser *parser) +if_block * +ifparser_getfirst (if_parser *parser) { - return parser->first; + return c_list_first_entry (&parser->block_lst_head, if_block, block_lst); } -int ifparser_get_num_blocks (if_parser *parser) +guint +ifparser_get_num_blocks (if_parser *parser) { - int i = 0; - if_block *iter = parser->first; - - while (iter) { - i++; - iter = iter->next; - } - return i; + return c_list_length (&parser->block_lst_head); } if_block * ifparser_getif (if_parser *parser, const char* iface) { - if_block *curr = parser->first; - while (curr != NULL) { - if ( nm_streq (curr->type, "iface") - && nm_streq (curr->name, iface)) - return curr; - curr = curr->next; + if_block *ifb; + + c_list_for_each_entry (ifb, &parser->block_lst_head, block_lst) { + if ( nm_streq (ifb->type, "iface") + && nm_streq (ifb->name, iface)) + return ifb; } return NULL; } -const char * -ifparser_getkey (if_block* iface, const char *key) +static if_data * +ifparser_findkey (if_block* iface, const char *key) { - if_data *curr = iface->info; - while (curr != NULL) { - if (nm_streq (curr->key, key)) - return curr->data; - curr = curr->next; + if_data *ifd; + + c_list_for_each_entry (ifd, &iface->data_lst_head, data_lst) { + if (nm_streq (ifd->key, key)) + return ifd; } return NULL; } +const char * +ifparser_getkey (if_block* iface, const char *key) +{ + if_data *ifd; + + ifd = ifparser_findkey (iface, key); + return ifd ? ifd->data : NULL; +} + gboolean ifparser_haskey (if_block* iface, const char *key) { - if_data *curr = iface->info; - - while (curr != NULL) { - if (nm_streq (curr->key, key)) - return TRUE; - curr = curr->next; - } - return FALSE; + return !!ifparser_findkey (iface, key); } -int +guint ifparser_get_num_info (if_block* iface) { - int i = 0; - if_data *iter = iface->info; - - while (iter) { - i++; - iter = iter->next; - } - return i; + return c_list_length (&iface->data_lst_head); } diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.h b/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.h index d6f1871109..44d6996a5f 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.h +++ b/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.h @@ -23,20 +23,24 @@ #ifndef _INTERFACE_PARSER_H #define _INTERFACE_PARSER_H -typedef struct _if_data { +#include "c-list/src/c-list.h" + +typedef struct { + CList data_lst; char *key; char *data; - struct _if_data *next; } if_data; -typedef struct _if_block { +typedef struct { + CList block_lst; + CList data_lst_head; char *type; char *name; - if_data *info; - struct _if_block *next; } if_block; -typedef struct _if_parser if_parser; +typedef struct { + CList block_lst_head; +} if_parser; if_parser *ifparser_parse (const char *eni_file, int quiet); @@ -48,7 +52,8 @@ if_block *ifparser_getif (if_parser *parser, const char* iface); if_block *ifparser_getfirst (if_parser *parser); const char *ifparser_getkey (if_block* iface, const char *key); gboolean ifparser_haskey (if_block* iface, const char *key); -int ifparser_get_num_blocks (if_parser *parser); -int ifparser_get_num_info (if_block* iface); + +guint ifparser_get_num_blocks (if_parser *parser); +guint ifparser_get_num_info (if_block* iface); #endif diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-parser.c b/src/settings/plugins/ifupdown/nms-ifupdown-parser.c index c86cf7520f..7270773c8e 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-parser.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-parser.c @@ -49,24 +49,24 @@ static const char* _ifupdownplugin_guess_connection_type (if_block *block) { - if_data *curr = block->info; - const char* ret_type = NULL; - const char* value = ifparser_getkey (block, "inet"); + const char *ret_type = NULL; - if (nm_streq0 (value, "ppp")) + if(nm_streq0 (ifparser_getkey (block, "inet"), "ppp")) ret_type = NM_SETTING_PPP_SETTING_NAME; - - while (!ret_type && curr) { - if ( _str_has_prefix (curr->key, "wireless-", FALSE) - || _str_has_prefix (curr->key, "wpa-", FALSE)) { - ret_type = NM_SETTING_WIRELESS_SETTING_NAME; + else { + if_data *ifb; + + c_list_for_each_entry (ifb, &block->data_lst_head, data_lst) { + if ( _str_has_prefix (ifb->key, "wireless-", FALSE) + || _str_has_prefix (ifb->key, "wpa-", FALSE)) { + ret_type = NM_SETTING_WIRELESS_SETTING_NAME; + break; + } } - curr = curr->next; + if(!ret_type) + ret_type = NM_SETTING_WIRED_SETTING_NAME; } - if (!ret_type) - ret_type = NM_SETTING_WIRED_SETTING_NAME; - nm_log_info (LOGD_SETTINGS, "guessed connection type (%s) = %s", block->name, ret_type); return ret_type; } @@ -93,8 +93,8 @@ static void update_wireless_setting_from_if_block (NMConnection *connection, if_block *block) { - if_data *curr = block->info; - const char* value = ifparser_getkey (block, "inet"); + if_data *curr; + const char *value = ifparser_getkey (block, "inet"); struct _Mapping mapping[] = { {"ssid", "ssid"}, {"essid", "ssid"}, @@ -110,7 +110,7 @@ update_wireless_setting_from_if_block (NMConnection *connection, nm_log_info (LOGD_SETTINGS, "update wireless settings (%s).", block->name); wireless_setting = NM_SETTING_WIRELESS (nm_setting_wireless_new ()); - while (curr) { + c_list_for_each_entry (curr, &block->data_lst_head, data_lst) { if (_str_has_prefix (curr->key, "wireless-", TRUE)) { const char* newkey = map_by_mapping (mapping, curr->key + NM_STRLEN ("wireless-")); @@ -156,7 +156,6 @@ update_wireless_setting_from_if_block (NMConnection *connection, nm_log_info (LOGD_SETTINGS, "setting wpa newkey(%s)=data(%s)", newkey, curr->data); } } - curr = curr->next; } nm_connection_add_setting (connection, (NMSetting*) wireless_setting); } @@ -248,7 +247,7 @@ static void update_wireless_security_setting_from_if_block (NMConnection *connection, if_block *block) { - if_data *curr = block->info; + if_data *curr; const char* value = ifparser_getkey (block, "inet"); struct _Mapping mapping[] = { {"psk", "psk"}, @@ -314,7 +313,7 @@ update_wireless_security_setting_from_if_block (NMConnection *connection, nm_log_info (LOGD_SETTINGS, "update wireless security settings (%s).", block->name); wireless_security_setting = NM_SETTING_WIRELESS_SECURITY (nm_setting_wireless_security_new ()); - while (curr) { + c_list_for_each_entry (curr, &block->data_lst_head, data_lst) { if (_str_has_prefix (curr->key, "wireless-", TRUE)) { const char *key = curr->key + NM_STRLEN ("wireless-"); char *property_value = NULL; @@ -391,7 +390,7 @@ wireless_next: (*free_func) (typed_property_value); } next: - curr = curr->next; + ; } if (security) diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c index 2e86d3f521..88c651b859 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c @@ -158,9 +158,9 @@ initialize (NMSettingsPlugin *plugin) const char *block_name; NMIfupdownConnection *conn; - /* Read in all the interfaces */ parser = ifparser_parse (ENI_INTERFACES_FILE, 0); - for (block = ifparser_getfirst (parser); block; block = block->next) { + + c_list_for_each_entry (block, &parser->block_lst_head, block_lst) { if (NM_IN_STRSET (block->type, "auto", "allow-hotplug")) { if (!auto_ifaces) diff --git a/src/settings/plugins/ifupdown/tests/test-ifupdown.c b/src/settings/plugins/ifupdown/tests/test-ifupdown.c index 634ec9af29..e96350c0fb 100644 --- a/src/settings/plugins/ifupdown/tests/test-ifupdown.c +++ b/src/settings/plugins/ifupdown/tests/test-ifupdown.c @@ -137,9 +137,8 @@ compare_expected_to_ifparser (if_parser *parser, Expected *e) g_assert_cmpint (g_slist_length (e->blocks), ==, ifparser_get_num_blocks (parser)); - for (n = ifparser_getfirst (parser), biter = e->blocks; - n && biter; - n = n->next, biter = g_slist_next (biter)) { + biter = e->blocks; + c_list_for_each_entry (n, &parser->block_lst_head, block_lst) { if_data *m; ExpectedBlock *b = biter->data; @@ -150,17 +149,22 @@ compare_expected_to_ifparser (if_parser *parser, Expected *e) g_assert_cmpint (g_slist_length (b->keys), ==, ifparser_get_num_info (n)); - for (m = n->info, kiter = b->keys; - m && kiter; - m = m->next, kiter = g_slist_next (kiter)) { + kiter = b->keys; + c_list_for_each_entry (m, &n->data_lst_head, data_lst) { ExpectedKey *k = kiter->data; g_assert (k->key && m->key); g_assert_cmpstr (k->key, ==, m->key); g_assert (k->data && m->data); g_assert_cmpstr (k->data, ==, m->data); + + kiter = g_slist_next (kiter); } + g_assert (!kiter); + + biter = g_slist_next (biter); } + g_assert (!biter); } static void @@ -169,7 +173,7 @@ dump_blocks (if_parser *parser) if_block *n; g_message ("\n***************************************************"); - for (n = ifparser_getfirst (parser); n != NULL; n = n->next) { + c_list_for_each_entry (n, &parser->block_lst_head, block_lst) { if_data *m; // each block start with its type & name @@ -178,8 +182,8 @@ dump_blocks (if_parser *parser) // each key-value pair within a block is indented & separated by a tab // (single quotes used to show typ & name baoundaries) - for (m = n->info; m != NULL; m = m->next) - g_print("\t'%s'\t'%s'\n", m->key, m->data); + c_list_for_each_entry (m, &n->data_lst_head, data_lst) + g_print("\t'%s'\t'%s'\n", m->key, m->data); // blocks are separated by an empty line g_print("\n"); @@ -465,7 +469,7 @@ test17_read_static_ipv4 (void) connection = nm_simple_connection_new(); g_assert (connection); - ifupdown_update_connection_from_if_block(connection, block, &error); + ifupdown_update_connection_from_if_block (connection, block, &error); g_assert_no_error (error); success = nm_connection_verify (connection, &error); @@ -519,7 +523,7 @@ test18_read_static_ipv6 (void) block = ifparser_getfirst (parser); connection = nm_simple_connection_new(); g_assert (connection); - ifupdown_update_connection_from_if_block(connection, block, &error); + ifupdown_update_connection_from_if_block (connection, block, &error); g_assert_no_error (error); success = nm_connection_verify (connection, &error); @@ -571,7 +575,7 @@ test19_read_static_ipv4_plen (void) block = ifparser_getfirst (parser); connection = nm_simple_connection_new(); g_assert (connection); - ifupdown_update_connection_from_if_block(connection, block, &error); + ifupdown_update_connection_from_if_block (connection, block, &error); g_assert_no_error (error); success = nm_connection_verify (connection, &error); -- cgit v1.2.1 From bb273c0881b39a1d930588014a2a0d2740272cfe Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 25 Aug 2018 22:34:25 +0200 Subject: settings/ifupdown: optimize allocating parser data --- .../ifupdown/nms-ifupdown-interface-parser.c | 32 +++++++++++++--------- .../ifupdown/nms-ifupdown-interface-parser.h | 8 +++--- .../plugins/ifupdown/nms-ifupdown-parser.c | 2 +- 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c b/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c index b833dacbd7..404cc83c1f 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c @@ -42,10 +42,15 @@ static void add_block (if_parser *parser, const char *type, const char* name) { if_block *ifb; + gsize l_type, l_name; - ifb = g_slice_new (if_block); - ifb->name = g_strdup (name); - ifb->type = g_strdup (type); + l_type = strlen (type) + 1; + l_name = strlen (name) + 1; + + ifb = g_malloc (sizeof (if_block) + l_type + l_name); + memcpy ((char *) ifb->name, name, l_name); + ifb->type = &ifb->name[l_name]; + memcpy ((char *) ifb->type, type, l_type); c_list_init (&ifb->data_lst_head); c_list_link_tail (&parser->block_lst_head, &ifb->block_lst); } @@ -56,6 +61,7 @@ add_data (if_parser *parser, const char *key, const char *data) if_block *last_block; if_data *ifd; char *idx; + gsize l_key, l_data; last_block = c_list_last_entry (&parser->block_lst_head, if_block, block_lst); @@ -63,13 +69,17 @@ add_data (if_parser *parser, const char *key, const char *data) if (!last_block) return; - ifd = g_slice_new (if_data); - ifd->key = g_strdup (key); - ifd->data = g_strdup (data); + l_key = strlen (key) + 1; + l_data = strlen (data) + 1; + + ifd = g_malloc (sizeof (if_data) + l_key + l_data); + memcpy ((char *) ifd->key, key, l_key); + ifd->data = &ifd->key[l_key]; + memcpy ((char *) ifd->data, data, l_data); /* Normalize keys. Convert '_' to '-', as ifupdown accepts both variants. * When querying keys via ifparser_getkey(), use '-'. */ - idx = ifd->key; + idx = (char *) ifd->key; while ((idx = strchr (idx, '_'))) *(idx++) = '-'; @@ -307,9 +317,7 @@ static void _destroy_data (if_data *ifd) { c_list_unlink_stale (&ifd->data_lst); - g_free (ifd->key); - g_free (ifd->data); - g_slice_free (if_data, ifd); + g_free (ifd); } static void @@ -320,9 +328,7 @@ _destroy_block (if_block* ifb) while ((ifd = c_list_first_entry (&ifb->data_lst_head, if_data, data_lst))) _destroy_data (ifd); c_list_unlink_stale (&ifb->block_lst); - g_free (ifb->name); - g_free (ifb->type); - g_slice_free (if_block, ifb); + g_free (ifb); } void diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.h b/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.h index 44d6996a5f..f367f62695 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.h +++ b/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.h @@ -27,15 +27,15 @@ typedef struct { CList data_lst; - char *key; - char *data; + const char *data; + const char key[]; } if_data; typedef struct { CList block_lst; CList data_lst_head; - char *type; - char *name; + const char *type; + const char name[]; } if_block; typedef struct { diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-parser.c b/src/settings/plugins/ifupdown/nms-ifupdown-parser.c index 7270773c8e..46eb2550a8 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-parser.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-parser.c @@ -160,7 +160,7 @@ update_wireless_setting_from_if_block (NMConnection *connection, nm_connection_add_setting (connection, (NMSetting*) wireless_setting); } -typedef char* (*IfupdownStrDupeFunc) (gpointer value, gpointer data); +typedef char* (*IfupdownStrDupeFunc) (gconstpointer value, gpointer data); typedef gpointer (*IfupdownStrToTypeFunc) (const char* value); static char* -- cgit v1.2.1 From 7064b81bbcca471af34f079a9c6d74689f1bea79 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 25 Aug 2018 23:14:20 +0200 Subject: settings/ifupdown: various cleanup in nms-ifupdown-parser.c --- .../plugins/ifupdown/nms-ifupdown-parser.c | 79 +++++++++------------- 1 file changed, 32 insertions(+), 47 deletions(-) diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-parser.c b/src/settings/plugins/ifupdown/nms-ifupdown-parser.c index 46eb2550a8..b5dde2a3ae 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-parser.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-parser.c @@ -384,12 +384,12 @@ wireless_next: NULL); security = TRUE; - wpa_next: +wpa_next: g_free (property_value); if (free_func && typed_property_value) (*free_func) (typed_property_value); } - next: +next: ; } @@ -410,15 +410,15 @@ static void ifupdown_ip4_add_dns (NMSettingIPConfig *s_ip4, const char *dns) { guint32 addr; - char **list, **iter; + gs_strfreev char **list = NULL; + char **iter; if (dns == NULL) return; list = g_strsplit_set (dns, " \t", -1); for (iter = list; iter && *iter; iter++) { - g_strstrip (*iter); - if (g_ascii_isspace (*iter[0])) + if ((*iter)[0] == '\0') continue; if (!inet_pton (AF_INET, *iter, &addr)) { nm_log_warn (LOGD_SETTINGS, " ignoring invalid nameserver '%s'", *iter); @@ -428,7 +428,6 @@ ifupdown_ip4_add_dns (NMSettingIPConfig *s_ip4, const char *dns) if (!nm_setting_ip_config_add_dns (s_ip4, *iter)) nm_log_warn (LOGD_SETTINGS, " duplicate DNS domain '%s'", *iter); } - g_strfreev (list); } static gboolean @@ -437,7 +436,7 @@ update_ip4_setting_from_if_block (NMConnection *connection, GError **error) { - NMSettingIPConfig *s_ip4 = NM_SETTING_IP_CONFIG (nm_setting_ip4_config_new ()); + gs_unref_object NMSettingIPConfig *s_ip4 = NM_SETTING_IP_CONFIG (nm_setting_ip4_config_new ()); const char *type = ifparser_getkey (block, "inet"); if (!nm_streq0 (type, "static")) { @@ -454,7 +453,6 @@ update_ip4_setting_from_if_block (NMConnection *connection, const char *nameserver_v; const char *nameservers_v; const char *search_v; - char **list, **iter; guint32 netmask_int = 32; /* Address */ @@ -462,7 +460,7 @@ update_ip4_setting_from_if_block (NMConnection *connection, if (!address_v) { g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION, "Missing IPv4 address"); - goto error; + return FALSE; } /* mask/prefix */ @@ -472,8 +470,8 @@ update_ip4_setting_from_if_block (NMConnection *connection, netmask_int = atoi (netmask_v); } else if (!inet_pton (AF_INET, netmask_v, &tmp_mask)) { g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION, - "Invalid IPv4 netmask '%s'", netmask_v); - goto error; + "Invalid IPv4 netmask '%s'", netmask_v); + return FALSE; } else { netmask_int = nm_utils_ip4_netmask_to_prefix (tmp_mask); } @@ -482,7 +480,7 @@ update_ip4_setting_from_if_block (NMConnection *connection, /* Add the new address to the setting */ addr = nm_ip_address_new (AF_INET, address_v, netmask_int, error); if (!addr) - goto error; + return FALSE; if (nm_setting_ip_config_add_address (s_ip4, addr)) { nm_log_info (LOGD_SETTINGS, "addresses count: %d", @@ -498,7 +496,7 @@ update_ip4_setting_from_if_block (NMConnection *connection, if (!nm_utils_ipaddr_valid (AF_INET, gateway_v)) { g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION, "Invalid IPv4 gateway '%s'", gateway_v); - goto error; + return FALSE; } if (!nm_setting_ip_config_get_gateway (s_ip4)) g_object_set (s_ip4, NM_SETTING_IP_CONFIG_GATEWAY, gateway_v, NULL); @@ -516,41 +514,38 @@ update_ip4_setting_from_if_block (NMConnection *connection, /* DNS searches */ search_v = ifparser_getkey (block, "dns-search"); if (search_v) { + gs_strfreev char **list = NULL; + char **iter; + list = g_strsplit_set (search_v, " \t", -1); for (iter = list; iter && *iter; iter++) { - g_strstrip (*iter); - if (g_ascii_isspace (*iter[0])) + if ((*iter)[0] == '\0') continue; if (!nm_setting_ip_config_add_dns_search (s_ip4, *iter)) nm_log_warn (LOGD_SETTINGS, " duplicate DNS domain '%s'", *iter); } - g_strfreev (list); } g_object_set (s_ip4, NM_SETTING_IP_CONFIG_METHOD, NM_SETTING_IP4_CONFIG_METHOD_MANUAL, NULL); } - nm_connection_add_setting (connection, NM_SETTING (s_ip4)); + nm_connection_add_setting (connection, NM_SETTING (g_steal_pointer (&s_ip4))); return TRUE; - -error: - g_object_unref (s_ip4); - return FALSE; } static void ifupdown_ip6_add_dns (NMSettingIPConfig *s_ip6, const char *dns) { struct in6_addr addr; - char **list, **iter; + gs_strfreev char **list = NULL; + char **iter; if (dns == NULL) return; list = g_strsplit_set (dns, " \t", -1); for (iter = list; iter && *iter; iter++) { - g_strstrip (*iter); - if (g_ascii_isspace (*iter[0])) + if ((*iter)[0] == '\0') continue; if (!inet_pton (AF_INET6, *iter, &addr)) { nm_log_warn (LOGD_SETTINGS, " ignoring invalid nameserver '%s'", *iter); @@ -560,7 +555,6 @@ ifupdown_ip6_add_dns (NMSettingIPConfig *s_ip6, const char *dns) if (!nm_setting_ip_config_add_dns (s_ip6, *iter)) nm_log_warn (LOGD_SETTINGS, " duplicate DNS domain '%s'", *iter); } - g_strfreev (list); } static gboolean @@ -568,7 +562,7 @@ update_ip6_setting_from_if_block (NMConnection *connection, if_block *block, GError **error) { - NMSettingIPConfig *s_ip6 = NM_SETTING_IP_CONFIG (nm_setting_ip6_config_new ()); + gs_unref_object NMSettingIPConfig *s_ip6 = NM_SETTING_IP_CONFIG (nm_setting_ip6_config_new ()); const char *type = ifparser_getkey (block, "inet6"); if (!NM_IN_STRSET (type, "static", "v4tunnel")) { @@ -585,14 +579,13 @@ update_ip6_setting_from_if_block (NMConnection *connection, const char *nameservers_v; const char *search_v; int prefix_int = 128; - char **list, **iter; /* Address */ address_v = ifparser_getkey (block, "address"); if (!address_v) { g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION, "Missing IPv6 address"); - goto error; + return FALSE; } /* Prefix */ @@ -603,7 +596,7 @@ update_ip6_setting_from_if_block (NMConnection *connection, /* Add the new address to the setting */ addr = nm_ip_address_new (AF_INET6, address_v, prefix_int, error); if (!addr) - goto error; + return FALSE; if (nm_setting_ip_config_add_address (s_ip6, addr)) { nm_log_info (LOGD_SETTINGS, "addresses count: %d", @@ -619,7 +612,7 @@ update_ip6_setting_from_if_block (NMConnection *connection, if (!nm_utils_ipaddr_valid (AF_INET6, gateway_v)) { g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION, "Invalid IPv6 gateway '%s'", gateway_v); - goto error; + return FALSE; } if (!nm_setting_ip_config_get_gateway (s_ip6)) g_object_set (s_ip6, NM_SETTING_IP_CONFIG_GATEWAY, gateway_v, NULL); @@ -637,15 +630,16 @@ update_ip6_setting_from_if_block (NMConnection *connection, /* DNS searches */ search_v = ifparser_getkey (block, "dns-search"); if (search_v) { + gs_strfreev char **list = NULL; + char **iter; + list = g_strsplit_set (search_v, " \t", -1); for (iter = list; iter && *iter; iter++) { - g_strstrip (*iter); - if (isblank (*iter[0])) + if ((*iter)[0] == '\0') continue; if (!nm_setting_ip_config_add_dns_search (s_ip6, *iter)) nm_log_warn (LOGD_SETTINGS, " duplicate DNS domain '%s'", *iter); } - g_strfreev (list); } g_object_set (s_ip6, @@ -653,12 +647,8 @@ update_ip6_setting_from_if_block (NMConnection *connection, NULL); } - nm_connection_add_setting (connection, NM_SETTING (s_ip6)); + nm_connection_add_setting (connection, NM_SETTING (g_steal_pointer (&s_ip6))); return TRUE; - -error: - g_object_unref (s_ip6); - return FALSE; } gboolean @@ -666,25 +656,22 @@ ifupdown_update_connection_from_if_block (NMConnection *connection, if_block *block, GError **error) { - const char *type = NULL; - char *idstr = NULL; - char *uuid_base = NULL; - char *uuid = NULL; + const char *type; + gs_free char *idstr = NULL; + gs_free char *uuid = NULL; NMSettingConnection *s_con; gboolean success = FALSE; s_con = nm_connection_get_setting_connection (connection); if (!s_con) { s_con = NM_SETTING_CONNECTION (nm_setting_connection_new ()); - g_assert (s_con); nm_connection_add_setting (connection, NM_SETTING (s_con)); } type = _ifupdownplugin_guess_connection_type (block); idstr = g_strconcat ("Ifupdown (", block->name, ")", NULL); - uuid_base = idstr; - uuid = nm_utils_uuid_generate_from_string (uuid_base, -1, NM_UTILS_UUID_TYPE_LEGACY, NULL); + uuid = nm_utils_uuid_generate_from_string (idstr, -1, NM_UTILS_UUID_TYPE_LEGACY, NULL); g_object_set (s_con, NM_SETTING_CONNECTION_TYPE, type, NM_SETTING_CONNECTION_INTERFACE_NAME, block->name, @@ -693,7 +680,6 @@ ifupdown_update_connection_from_if_block (NMConnection *connection, NM_SETTING_CONNECTION_READ_ONLY, TRUE, NM_SETTING_CONNECTION_AUTOCONNECT, FALSE, NULL); - g_free (uuid); nm_log_info (LOGD_SETTINGS, "update_connection_setting_from_if_block: name:%s, type:%s, id:%s, uuid: %s", block->name, type, idstr, nm_setting_connection_get_uuid (s_con)); @@ -713,6 +699,5 @@ ifupdown_update_connection_from_if_block (NMConnection *connection, if (success == TRUE) success = nm_connection_verify (connection, error); - g_free (idstr); return success; } -- cgit v1.2.1 From 3091ffa50ada26296d8facf3935069fb5bf39e19 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 26 Aug 2018 10:02:28 +0200 Subject: settings/ifupdown: use _NMLOG() macros for logging Does not address the issues that the existing logging is much too verbose and does not provide necessary context for what it's complaining. The logging messages should be improved. At least, the logging macro gives all messages a "ifupdown: " prefix. --- .../plugins/ifupdown/nms-ifupdown-connection.c | 18 +++-- .../ifupdown/nms-ifupdown-interface-parser.c | 40 ++++++---- .../plugins/ifupdown/nms-ifupdown-parser.c | 88 ++++++++++++---------- 3 files changed, 88 insertions(+), 58 deletions(-) diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-connection.c b/src/settings/plugins/ifupdown/nms-ifupdown-connection.c index bdca91d66c..1b81704420 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-connection.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-connection.c @@ -49,10 +49,20 @@ G_DEFINE_TYPE (NMIfupdownConnection, nm_ifupdown_connection, NM_TYPE_SETTINGS_CO /*****************************************************************************/ +#define _NMLOG_PREFIX_NAME "ifupdown" +#define _NMLOG_DOMAIN LOGD_SETTINGS +#define _NMLOG(level, ...) \ + nm_log ((level), _NMLOG_DOMAIN, NULL, NULL, \ + "%s" _NM_UTILS_MACRO_FIRST (__VA_ARGS__), \ + _NMLOG_PREFIX_NAME": " \ + _NM_UTILS_MACRO_REST (__VA_ARGS__)) + +/*****************************************************************************/ + static gboolean supports_secrets (NMSettingsConnection *connection, const char *setting_name) { - nm_log_info (LOGD_SETTINGS, "supports_secrets() for setting_name: '%s'", setting_name); + _LOGI ("supports_secrets() for setting_name: '%s'", setting_name); return (strcmp (setting_name, NM_SETTING_WIRELESS_SECURITY_SETTING_NAME) == 0); } @@ -78,10 +88,8 @@ nm_ifupdown_connection_new (if_block *block) if (!ifupdown_update_connection_from_if_block (nm_settings_connection_get_connection (NM_SETTINGS_CONNECTION (connection)), block, &error)) { - nm_log_warn (LOGD_SETTINGS, "%s.%d - invalid connection read from /etc/network/interfaces: %s", - __FILE__, - __LINE__, - error->message); + _LOGW ("invalid connection read from /etc/network/interfaces: %s", + error->message); g_object_unref (connection); return NULL; } diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c b/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c index 404cc83c1f..d926829fb1 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c @@ -38,6 +38,16 @@ static void _ifparser_source (if_parser *parser, const char *path, const char *e /*****************************************************************************/ +#define _NMLOG_PREFIX_NAME "ifupdown" +#define _NMLOG_DOMAIN LOGD_SETTINGS +#define _NMLOG(level, ...) \ + nm_log ((level), _NMLOG_DOMAIN, NULL, NULL, \ + "%s" _NM_UTILS_MACRO_FIRST (__VA_ARGS__), \ + _NMLOG_PREFIX_NAME": " \ + _NM_UTILS_MACRO_REST (__VA_ARGS__)) + +/*****************************************************************************/ + static void add_block (if_parser *parser, const char *type, const char* name) { @@ -116,17 +126,17 @@ _recursive_ifparser (if_parser *parser, const char *eni_file, int quiet) /* Check if interfaces file exists and open it */ if (!g_file_test (eni_file, G_FILE_TEST_EXISTS)) { if (!quiet) - nm_log_warn (LOGD_SETTINGS, "interfaces file %s doesn't exist\n", eni_file); + _LOGW ("interfaces file %s doesn't exist\n", eni_file); return; } inp = fopen (eni_file, "re"); if (inp == NULL) { if (!quiet) - nm_log_warn (LOGD_SETTINGS, "Can't open %s\n", eni_file); + _LOGW ("Can't open %s\n", eni_file); return; } if (!quiet) - nm_log_info (LOGD_SETTINGS, " interface-parser: parsing file %s\n", eni_file); + _LOGI (" interface-parser: parsing file %s\n", eni_file); while (!feof (inp)) { char *token[128]; /* 255 chars can only be split into 127 tokens */ @@ -144,7 +154,7 @@ _recursive_ifparser (if_parser *parser, const char *eni_file, int quiet) if (!feof (inp) && len > 0 && line[len-1] != '\n') { if (!skip_long_line) { if (!quiet) - nm_log_warn (LOGD_SETTINGS, "Skipping over-long-line '%s...'\n", line); + _LOGW ("Skipping over-long-line '%s...'\n", line); } skip_long_line = 1; continue; @@ -182,8 +192,8 @@ _recursive_ifparser (if_parser *parser, const char *eni_file, int quiet) if (toknum < 2) { if (!quiet) { - nm_log_warn (LOGD_SETTINGS, "Can't parse interface line '%s'\n", - join_values_with_spaces (value, token)); + _LOGW ("Can't parse interface line '%s'\n", + join_values_with_spaces (value, token)); } skip_to_block = 1; continue; @@ -197,8 +207,8 @@ _recursive_ifparser (if_parser *parser, const char *eni_file, int quiet) if (nm_streq (token[0], "iface")) { if (toknum < 4) { if (!quiet) { - nm_log_warn (LOGD_SETTINGS, "Can't parse iface line '%s'\n", - join_values_with_spaces (value, token)); + _LOGW ("Can't parse iface line '%s'\n", + join_values_with_spaces (value, token)); } continue; } @@ -244,8 +254,8 @@ _recursive_ifparser (if_parser *parser, const char *eni_file, int quiet) else { if (skip_to_block) { if (!quiet) { - nm_log_warn (LOGD_SETTINGS, "ignoring out-of-block data '%s'\n", - join_values_with_spaces (value, token)); + _LOGW ("ignoring out-of-block data '%s'\n", + join_values_with_spaces (value, token)); } } else add_data (parser, token[0], join_values_with_spaces (value, token + 1)); @@ -254,7 +264,7 @@ _recursive_ifparser (if_parser *parser, const char *eni_file, int quiet) fclose (inp); if (!quiet) - nm_log_info (LOGD_SETTINGS, " interface-parser: finished parsing file %s\n", eni_file); + _LOGI (" interface-parser: finished parsing file %s\n", eni_file); } static void @@ -273,20 +283,20 @@ _ifparser_source (if_parser *parser, const char *path, const char *en_dir, int q abs_path = g_build_filename (en_dir, path, NULL); if (!quiet) - nm_log_info (LOGD_SETTINGS, " interface-parser: source line includes interfaces file(s) %s\n", abs_path); + _LOGI (" interface-parser: source line includes interfaces file(s) %s\n", abs_path); /* ifupdown uses WRDE_NOCMD for wordexp. */ if (wordexp (abs_path, &we, WRDE_NOCMD)) { if (!quiet) - nm_log_warn (LOGD_SETTINGS, "word expansion for %s failed\n", abs_path); + _LOGW ("word expansion for %s failed\n", abs_path); } else { for (i = 0; i < we.we_wordc; i++) { if (dir) { source_dir = g_dir_open (we.we_wordv[i], 0, &error); if (!source_dir) { if (!quiet) { - nm_log_warn (LOGD_SETTINGS, "Failed to open directory %s: %s", - we.we_wordv[i], error->message); + _LOGW ("Failed to open directory %s: %s", + we.we_wordv[i], error->message); } g_clear_error (&error); } else { diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-parser.c b/src/settings/plugins/ifupdown/nms-ifupdown-parser.c index b5dde2a3ae..369fa70df2 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-parser.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-parser.c @@ -37,6 +37,18 @@ #include "nms-ifupdown-plugin.h" #include "nms-ifupdown-parser.h" +/*****************************************************************************/ + +#define _NMLOG_PREFIX_NAME "ifupdown" +#define _NMLOG_DOMAIN LOGD_SETTINGS +#define _NMLOG(level, ...) \ + nm_log ((level), _NMLOG_DOMAIN, NULL, NULL, \ + "%s" _NM_UTILS_MACRO_FIRST (__VA_ARGS__), \ + _NMLOG_PREFIX_NAME": " \ + _NM_UTILS_MACRO_REST (__VA_ARGS__)) + +/*****************************************************************************/ + #define _str_has_prefix(val, prefix, require_suffix) \ ({ \ const char *_val = (val); \ @@ -67,7 +79,7 @@ _ifupdownplugin_guess_connection_type (if_block *block) ret_type = NM_SETTING_WIRED_SETTING_NAME; } - nm_log_info (LOGD_SETTINGS, "guessed connection type (%s) = %s", block->name, ret_type); + _LOGI ("guessed connection type (%s) = %s", block->name, ret_type); return ret_type; } @@ -107,14 +119,14 @@ update_wireless_setting_from_if_block (NMConnection *connection, if (nm_streq0 (value, "ppp")) return; - nm_log_info (LOGD_SETTINGS, "update wireless settings (%s).", block->name); + _LOGI ("update wireless settings (%s).", block->name); wireless_setting = NM_SETTING_WIRELESS (nm_setting_wireless_new ()); c_list_for_each_entry (curr, &block->data_lst_head, data_lst) { if (_str_has_prefix (curr->key, "wireless-", TRUE)) { const char* newkey = map_by_mapping (mapping, curr->key + NM_STRLEN ("wireless-")); - nm_log_info (LOGD_SETTINGS, "wireless setting key: %s='%s'", newkey, curr->data); + _LOGI ("wireless setting key: %s='%s'", newkey, curr->data); if (nm_streq0 (newkey, "ssid")) { GBytes *ssid; int len = strlen (curr->data); @@ -122,7 +134,7 @@ update_wireless_setting_from_if_block (NMConnection *connection, ssid = g_bytes_new (curr->data, len); g_object_set (wireless_setting, NM_SETTING_WIRELESS_SSID, ssid, NULL); g_bytes_unref (ssid); - nm_log_info (LOGD_SETTINGS, "setting wireless ssid = %d", len); + _LOGI ("setting wireless ssid = %d", len); } else if (nm_streq0 (newkey, "mode")) { if (!g_ascii_strcasecmp (curr->data, "Managed") || !g_ascii_strcasecmp (curr->data, "Auto")) g_object_set (wireless_setting, NM_SETTING_WIRELESS_MODE, NM_SETTING_WIRELESS_MODE_INFRA, NULL); @@ -131,7 +143,7 @@ update_wireless_setting_from_if_block (NMConnection *connection, else if (!g_ascii_strcasecmp (curr->data, "Master")) g_object_set (wireless_setting, NM_SETTING_WIRELESS_MODE, NM_SETTING_WIRELESS_MODE_AP, NULL); else - nm_log_warn (LOGD_SETTINGS, "Invalid mode '%s' (not 'Ad-Hoc', 'Ap', 'Managed', or 'Auto')", curr->data); + _LOGW ("Invalid mode '%s' (not 'Ad-Hoc', 'Ap', 'Managed', or 'Auto')", curr->data); } else { g_object_set (wireless_setting, newkey, curr->data, @@ -147,13 +159,13 @@ update_wireless_setting_from_if_block (NMConnection *connection, ssid = g_bytes_new (curr->data, len); g_object_set (wireless_setting, NM_SETTING_WIRELESS_SSID, ssid, NULL); g_bytes_unref (ssid); - nm_log_info (LOGD_SETTINGS, "setting wpa ssid = %d", len); + _LOGI ("setting wpa ssid = %d", len); } else if (newkey) { g_object_set (wireless_setting, newkey, curr->data, NULL); - nm_log_info (LOGD_SETTINGS, "setting wpa newkey(%s)=data(%s)", newkey, curr->data); + _LOGI ("setting wpa newkey(%s)=data(%s)", newkey, curr->data); } } } @@ -310,7 +322,7 @@ update_wireless_security_setting_from_if_block (NMConnection *connection, s_wireless = nm_connection_get_setting_wireless (connection); g_return_if_fail (s_wireless); - nm_log_info (LOGD_SETTINGS, "update wireless security settings (%s).", block->name); + _LOGI ("update wireless security settings (%s).", block->name); wireless_security_setting = NM_SETTING_WIRELESS_SECURITY (nm_setting_wireless_security_new ()); c_list_for_each_entry (curr, &block->data_lst_head, data_lst) { @@ -326,8 +338,8 @@ update_wireless_security_setting_from_if_block (NMConnection *connection, goto next; property_value = (*dupe_func) (curr->data, connection); - nm_log_info (LOGD_SETTINGS, "setting wireless security key: %s=%s", - newkey, property_value); + _LOGI ("setting wireless security key: %s=%s", + newkey, property_value); if (type_map_func) { errno = 0; @@ -358,19 +370,19 @@ wireless_next: goto next; property_value = (*dupe_func) (curr->data, connection); - nm_log_info (LOGD_SETTINGS, "setting wpa security key: %s=%s", - newkey, - NM_IN_STRSET (newkey, "key", - "leap-password", - "pin", - "psk", - "wep-key0", - "wep-key1", - "wep-key2", - "wep-key3") - ? "" - : property_value - ); + _LOGI ("setting wpa security key: %s=%s", + newkey, + NM_IN_STRSET (newkey, "key", + "leap-password", + "pin", + "psk", + "wep-key0", + "wep-key1", + "wep-key2", + "wep-key3") + ? "" + : property_value + ); if (type_map_func) { errno = 0; @@ -421,12 +433,12 @@ ifupdown_ip4_add_dns (NMSettingIPConfig *s_ip4, const char *dns) if ((*iter)[0] == '\0') continue; if (!inet_pton (AF_INET, *iter, &addr)) { - nm_log_warn (LOGD_SETTINGS, " ignoring invalid nameserver '%s'", *iter); + _LOGW (" ignoring invalid nameserver '%s'", *iter); continue; } if (!nm_setting_ip_config_add_dns (s_ip4, *iter)) - nm_log_warn (LOGD_SETTINGS, " duplicate DNS domain '%s'", *iter); + _LOGW (" duplicate DNS domain '%s'", *iter); } } @@ -483,10 +495,10 @@ update_ip4_setting_from_if_block (NMConnection *connection, return FALSE; if (nm_setting_ip_config_add_address (s_ip4, addr)) { - nm_log_info (LOGD_SETTINGS, "addresses count: %d", - nm_setting_ip_config_get_num_addresses (s_ip4)); + _LOGI ("addresses count: %d", + nm_setting_ip_config_get_num_addresses (s_ip4)); } else { - nm_log_info (LOGD_SETTINGS, "ignoring duplicate IP4 address"); + _LOGI ("ignoring duplicate IP4 address"); } nm_ip_address_unref (addr); @@ -509,7 +521,7 @@ update_ip4_setting_from_if_block (NMConnection *connection, ifupdown_ip4_add_dns (s_ip4, nameservers_v); if (!nm_setting_ip_config_get_num_dns (s_ip4)) - nm_log_info (LOGD_SETTINGS, "No dns-nameserver configured in /etc/network/interfaces"); + _LOGI ("No dns-nameserver configured in /etc/network/interfaces"); /* DNS searches */ search_v = ifparser_getkey (block, "dns-search"); @@ -522,7 +534,7 @@ update_ip4_setting_from_if_block (NMConnection *connection, if ((*iter)[0] == '\0') continue; if (!nm_setting_ip_config_add_dns_search (s_ip4, *iter)) - nm_log_warn (LOGD_SETTINGS, " duplicate DNS domain '%s'", *iter); + _LOGW (" duplicate DNS domain '%s'", *iter); } } @@ -548,12 +560,12 @@ ifupdown_ip6_add_dns (NMSettingIPConfig *s_ip6, const char *dns) if ((*iter)[0] == '\0') continue; if (!inet_pton (AF_INET6, *iter, &addr)) { - nm_log_warn (LOGD_SETTINGS, " ignoring invalid nameserver '%s'", *iter); + _LOGW (" ignoring invalid nameserver '%s'", *iter); continue; } if (!nm_setting_ip_config_add_dns (s_ip6, *iter)) - nm_log_warn (LOGD_SETTINGS, " duplicate DNS domain '%s'", *iter); + _LOGW (" duplicate DNS domain '%s'", *iter); } } @@ -599,10 +611,10 @@ update_ip6_setting_from_if_block (NMConnection *connection, return FALSE; if (nm_setting_ip_config_add_address (s_ip6, addr)) { - nm_log_info (LOGD_SETTINGS, "addresses count: %d", + _LOGI ("addresses count: %d", nm_setting_ip_config_get_num_addresses (s_ip6)); } else { - nm_log_info (LOGD_SETTINGS, "ignoring duplicate IP6 address"); + _LOGI ("ignoring duplicate IP6 address"); } nm_ip_address_unref (addr); @@ -625,7 +637,7 @@ update_ip6_setting_from_if_block (NMConnection *connection, ifupdown_ip6_add_dns (s_ip6, nameservers_v); if (!nm_setting_ip_config_get_num_dns (s_ip6)) - nm_log_info (LOGD_SETTINGS, "No dns-nameserver configured in /etc/network/interfaces"); + _LOGI ("No dns-nameserver configured in /etc/network/interfaces"); /* DNS searches */ search_v = ifparser_getkey (block, "dns-search"); @@ -638,7 +650,7 @@ update_ip6_setting_from_if_block (NMConnection *connection, if ((*iter)[0] == '\0') continue; if (!nm_setting_ip_config_add_dns_search (s_ip6, *iter)) - nm_log_warn (LOGD_SETTINGS, " duplicate DNS domain '%s'", *iter); + _LOGW (" duplicate DNS domain '%s'", *iter); } } @@ -681,8 +693,8 @@ ifupdown_update_connection_from_if_block (NMConnection *connection, NM_SETTING_CONNECTION_AUTOCONNECT, FALSE, NULL); - nm_log_info (LOGD_SETTINGS, "update_connection_setting_from_if_block: name:%s, type:%s, id:%s, uuid: %s", - block->name, type, idstr, nm_setting_connection_get_uuid (s_con)); + _LOGI ("update_connection_setting_from_if_block: name:%s, type:%s, id:%s, uuid: %s", + block->name, type, idstr, nm_setting_connection_get_uuid (s_con)); if (nm_streq (type, NM_SETTING_WIRED_SETTING_NAME)) update_wired_setting_from_if_block (connection, block); -- cgit v1.2.1