From 61c6ccaad4cfd0c8dae8ea09707c4c4a9cc8c7e8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 25 Nov 2016 15:07:19 +0100 Subject: [PATCH] config: drop nm_config_get_dhcp_client() and access config directly Also, ifnet plugin would read the configuration value, which is just wrong because: - the configuration might not be set and ifnet would fail to fallback to the compile time default. - the configuration only is in effect if the plugin is also available. Otherwise, we fallback to the next plugin. Only the dhcp-manager knows which DHCP plugin is in use. --- src/NetworkManager.ver-orig | 3 +- src/dhcp/nm-dhcp-manager.c | 18 ++++++++- src/dhcp/nm-dhcp-manager.h | 2 + src/nm-config-data.c | 2 +- src/nm-config.c | 12 ------ src/nm-config.h | 2 +- src/nm-iface-helper.c | 38 ++++++++++--------- .../plugins/ifnet/nms-ifnet-net-utils.c | 18 +++++---- src/settings/plugins/ifnet/tests/test-ifnet.c | 10 ++--- src/tests/config/test-config.c | 17 +++++++-- 10 files changed, 73 insertions(+), 49 deletions(-) diff --git a/src/NetworkManager.ver-orig b/src/NetworkManager.ver-orig index e89ed95e0d..5b62f41b08 100644 --- a/src/NetworkManager.ver-orig +++ b/src/NetworkManager.ver-orig @@ -31,7 +31,6 @@ global: nm_config_get; nm_config_get_data; nm_config_get_data_orig; - nm_config_get_dhcp_client; nm_config_get_monitor_connection_files; nm_connection_add_setting; nm_connection_compare; @@ -127,6 +126,8 @@ global: nm_device_state_changed; nm_device_take_down; nm_device_uses_assumed_connection; + nm_dhcp_manager_get; + nm_dhcp_manager_get_config; nm_ethernet_address_is_valid; nm_exported_object_class_add_interface; nm_exported_object_export; diff --git a/src/dhcp/nm-dhcp-manager.c b/src/dhcp/nm-dhcp-manager.c index bce24a87f3..c78a3841ed 100644 --- a/src/dhcp/nm-dhcp-manager.c +++ b/src/dhcp/nm-dhcp-manager.c @@ -322,6 +322,17 @@ nm_dhcp_manager_get_lease_ip_configs (NMDhcpManager *self, return NULL; } +const char * +nm_dhcp_manager_get_config (NMDhcpManager *self) +{ + const NMDhcpClientFactory *factory; + + g_return_val_if_fail (NM_IS_DHCP_MANAGER (self), NULL); + + factory = NM_DHCP_MANAGER_GET_PRIVATE (self)->client_factory; + return factory ? factory->name : NULL; +} + /*****************************************************************************/ NM_DEFINE_SINGLETON_GETTER (NMDhcpManager, nm_dhcp_manager_get, NM_TYPE_DHCP_MANAGER); @@ -331,6 +342,7 @@ nm_dhcp_manager_init (NMDhcpManager *self) { NMDhcpManagerPrivate *priv = NM_DHCP_MANAGER_GET_PRIVATE (self); NMConfig *config = nm_config_get (); + gs_free char *client_free = NULL; const char *client; int i; const NMDhcpClientFactory *client_factory = NULL; @@ -347,7 +359,11 @@ nm_dhcp_manager_init (NMDhcpManager *self) } /* Client-specific setup */ - client = nm_config_get_dhcp_client (config); + client_free = nm_config_data_get_value (nm_config_get_data_orig (config), + NM_CONFIG_KEYFILE_GROUP_MAIN, + NM_CONFIG_KEYFILE_KEY_MAIN_DHCP, + NM_CONFIG_GET_VALUE_STRIP | NM_CONFIG_GET_VALUE_NO_EMPTY); + client = client_free; if (nm_config_get_configure_and_quit (config)) { client_factory = &_nm_dhcp_client_factory_internal; if (client && !nm_streq (client, client_factory->name)) diff --git a/src/dhcp/nm-dhcp-manager.h b/src/dhcp/nm-dhcp-manager.h index a948b60970..66fdd145db 100644 --- a/src/dhcp/nm-dhcp-manager.h +++ b/src/dhcp/nm-dhcp-manager.h @@ -40,6 +40,8 @@ GType nm_dhcp_manager_get_type (void); NMDhcpManager *nm_dhcp_manager_get (void); +const char *nm_dhcp_manager_get_config (NMDhcpManager *self); + void nm_dhcp_manager_set_default_hostname (NMDhcpManager *manager, const char *hostname); diff --git a/src/nm-config-data.c b/src/nm-config-data.c index 99f31c2237..0d554e55ea 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -569,7 +569,7 @@ static struct { { NM_CONFIG_KEYFILE_GROUP_MAIN, "plugins", NM_CONFIG_DEFAULT_MAIN_PLUGINS }, { NM_CONFIG_KEYFILE_GROUP_MAIN, "rc-manager", NM_CONFIG_DEFAULT_MAIN_RC_MANAGER }, { NM_CONFIG_KEYFILE_GROUP_MAIN, NM_CONFIG_KEYFILE_KEY_MAIN_AUTH_POLKIT, NM_CONFIG_DEFAULT_MAIN_AUTH_POLKIT }, - { NM_CONFIG_KEYFILE_GROUP_MAIN, "dhcp", NM_CONFIG_DEFAULT_MAIN_DHCP }, + { NM_CONFIG_KEYFILE_GROUP_MAIN, NM_CONFIG_KEYFILE_KEY_MAIN_DHCP, NM_CONFIG_DEFAULT_MAIN_DHCP }, { NM_CONFIG_KEYFILE_GROUP_LOGGING, "backend", NM_CONFIG_DEFAULT_LOGGING_BACKEND }, { NM_CONFIG_KEYFILE_GROUP_LOGGING, "audit", NM_CONFIG_DEFAULT_LOGGING_AUDIT }, }; diff --git a/src/nm-config.c b/src/nm-config.c index 4e63b2e9e0..774cb1b748 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -92,7 +92,6 @@ typedef struct { char *intern_config_file; gboolean monitor_connection_files; - char *dhcp_client; char *log_level; char *log_domains; @@ -266,14 +265,6 @@ nm_config_get_monitor_connection_files (NMConfig *config) return NM_CONFIG_GET_PRIVATE (config)->monitor_connection_files; } -const char * -nm_config_get_dhcp_client (NMConfig *config) -{ - g_return_val_if_fail (config != NULL, NULL); - - return NM_CONFIG_GET_PRIVATE (config)->dhcp_client; -} - const char * nm_config_get_log_level (NMConfig *config) { @@ -2322,8 +2313,6 @@ init_sync (GInitable *initable, GCancellable *cancellable, GError **error) priv->monitor_connection_files = nm_config_keyfile_get_boolean (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "monitor-connection-files", FALSE); - priv->dhcp_client = nm_strstrip (g_key_file_get_string (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "dhcp", NULL)); - priv->log_level = nm_strstrip (g_key_file_get_string (keyfile, NM_CONFIG_KEYFILE_GROUP_LOGGING, "level", NULL)); priv->log_domains = nm_strstrip (g_key_file_get_string (keyfile, NM_CONFIG_KEYFILE_GROUP_LOGGING, "domains", NULL)); @@ -2383,7 +2372,6 @@ finalize (GObject *gobject) g_free (priv->system_config_dir); g_free (priv->no_auto_default_file); g_free (priv->intern_config_file); - g_free (priv->dhcp_client); g_free (priv->log_level); g_free (priv->log_domains); g_free (priv->debug); diff --git a/src/nm-config.h b/src/nm-config.h index a229a17ff7..ec15f3c3b5 100644 --- a/src/nm-config.h +++ b/src/nm-config.h @@ -60,6 +60,7 @@ #define NM_CONFIG_KEYFILE_GROUP_IFNET "ifnet" #define NM_CONFIG_KEYFILE_KEY_MAIN_AUTH_POLKIT "auth-polkit" +#define NM_CONFIG_KEYFILE_KEY_MAIN_DHCP "dhcp" #define NM_CONFIG_KEYFILE_KEY_LOGGING_BACKEND "backend" #define NM_CONFIG_KEYFILE_KEY_CONFIG_ENABLE "enable" #define NM_CONFIG_KEYFILE_KEY_ATOMIC_SECTION_WAS ".was" @@ -116,7 +117,6 @@ NMConfigData *nm_config_get_data_orig (NMConfig *config); #define NM_CONFIG_GET_DATA_ORIG (nm_config_get_data_orig (nm_config_get ())) gboolean nm_config_get_monitor_connection_files (NMConfig *config); -const char *nm_config_get_dhcp_client (NMConfig *config); const char *nm_config_get_log_level (NMConfig *config); const char *nm_config_get_log_domains (NMConfig *config); const char *nm_config_get_debug (NMConfig *config); diff --git a/src/nm-iface-helper.c b/src/nm-iface-helper.c index b9498d3172..aa14b085fe 100644 --- a/src/nm-iface-helper.c +++ b/src/nm-iface-helper.c @@ -557,52 +557,56 @@ const NMDhcpClientFactory *const _nm_dhcp_manager_factories[3] = { /*****************************************************************************/ /* Stub functions */ +#include "nm-config.h" +#include "devices/nm-device.h" +#include "nm-active-connection.h" +#include "nm-bus-manager.h" + void nm_main_config_reload (int signal) { _LOGI (LOGD_CORE, "reloading configuration not supported"); } -gconstpointer nm_config_get (void); -const char *nm_config_get_dhcp_client (gpointer unused); -gboolean nm_config_get_configure_and_quit (gpointer unused); -gconstpointer nm_bus_manager_get (void); -void nm_bus_manager_register_object (gpointer unused, gpointer object); -void nm_bus_manager_unregister_object (gpointer unused, gpointer object); -GType nm_device_get_type (void); -GType nm_active_connection_get_type (void); - -gconstpointer +NMConfig * nm_config_get (void) { return GUINT_TO_POINTER (1); } -const char * -nm_config_get_dhcp_client (gpointer unused) +NMConfigData * +nm_config_get_data_orig (NMConfig *config) { - return "internal"; + return GUINT_TO_POINTER (1); +} + +char * +nm_config_data_get_value (const NMConfigData *config_data, const char *group, const char *key, NMConfigGetValueFlags flags) +{ + return NULL; } gboolean -nm_config_get_configure_and_quit (gpointer unused) +nm_config_get_configure_and_quit (NMConfig *config) { return TRUE; } -gconstpointer +NMBusManager * nm_bus_manager_get (void) { return GUINT_TO_POINTER (1); } void -nm_bus_manager_register_object (gpointer unused, gpointer object) +nm_bus_manager_register_object (NMBusManager *bus_manager, + GDBusObjectSkeleton *object) { } void -nm_bus_manager_unregister_object (gpointer unused, gpointer object) +nm_bus_manager_unregister_object (NMBusManager *bus_manager, + GDBusObjectSkeleton *object) { } diff --git a/src/settings/plugins/ifnet/nms-ifnet-net-utils.c b/src/settings/plugins/ifnet/nms-ifnet-net-utils.c index db3c642ad1..6531a23813 100644 --- a/src/settings/plugins/ifnet/nms-ifnet-net-utils.c +++ b/src/settings/plugins/ifnet/nms-ifnet-net-utils.c @@ -32,6 +32,7 @@ #include "NetworkManagerUtils.h" #include "settings/nm-settings-plugin.h" #include "nm-config.h" +#include "dhcp/nm-dhcp-manager.h" #include "nms-ifnet-wpa-parser.h" #include "nms-ifnet-net-parser.h" @@ -729,24 +730,25 @@ get_dhcp_hostname_and_client_id (char **hostname, char **client_id) *hostname = NULL; *client_id = NULL; - dhcp_client = nm_config_get_dhcp_client (nm_config_get ()); + dhcp_client = nm_dhcp_manager_get_config (nm_dhcp_manager_get ()); if (dhcp_client) { if (!strcmp (dhcp_client, "dhclient")) { g_file_get_contents (dhclient_conf, &contents, NULL, - NULL); + NULL); use_dhclient = TRUE; - } else if (!strcmp (dhcp_client, "dhcpcd")) + } else if (!strcmp (dhcp_client, "dhcpcd")) { g_file_get_contents (dhcpcd_conf, &contents, NULL, - NULL); + NULL); + } } else { if (g_file_test (dhclient_conf, G_FILE_TEST_IS_REGULAR)) { g_file_get_contents (dhclient_conf, &contents, NULL, - NULL); + NULL); use_dhclient = TRUE; - } - else if (g_file_test (dhcpcd_conf, G_FILE_TEST_IS_REGULAR)) + } else if (g_file_test (dhcpcd_conf, G_FILE_TEST_IS_REGULAR)) { g_file_get_contents (dhcpcd_conf, &contents, NULL, - NULL); + NULL); + } } if (!contents) return; diff --git a/src/settings/plugins/ifnet/tests/test-ifnet.c b/src/settings/plugins/ifnet/tests/test-ifnet.c index 93727af72a..68f2b928e3 100644 --- a/src/settings/plugins/ifnet/tests/test-ifnet.c +++ b/src/settings/plugins/ifnet/tests/test-ifnet.c @@ -30,8 +30,8 @@ #include "nm-utils.h" -#include "nm-config.h" #include "platform/nm-linux-platform.h" +#include "dhcp/nm-dhcp-manager.h" #include "settings/plugins/ifnet/nms-ifnet-net-parser.h" #include "settings/plugins/ifnet/nms-ifnet-net-utils.h" @@ -40,17 +40,17 @@ #include "nm-test-utils-core.h" -/* Fake NMConfig handling; the values it returns don't matter, so this +/* Fake config handling; the values it returns don't matter, so this * is easier than forcing it to read our own config file, etc. */ -NMConfig * -nm_config_get (void) +NMDhcpManager * +nm_dhcp_manager_get (void) { return NULL; } const char * -nm_config_get_dhcp_client (NMConfig *config) +nm_dhcp_manager_get_config (NMDhcpManager *dhcp_manager) { return "dhclient"; } diff --git a/src/tests/config/test-config.c b/src/tests/config/test-config.c index b05e2f1995..dd2a58a06a 100644 --- a/src/tests/config/test-config.c +++ b/src/tests/config/test-config.c @@ -46,6 +46,17 @@ _assert_config_value (const NMConfigData *config_data, const char *group, const } #define assert_config_value(config_data, group, key, expected_value) _assert_config_value (config_data, group, key, expected_value, __FILE__, __LINE__) +#define _config_get_dhcp_client_a(config) \ + ({ \ + gs_free char *_s = NULL; \ + \ + _s = nm_config_data_get_value (nm_config_get_data_orig (config), \ + NM_CONFIG_KEYFILE_GROUP_MAIN, \ + NM_CONFIG_KEYFILE_KEY_MAIN_DHCP, \ + NM_CONFIG_GET_VALUE_STRIP | NM_CONFIG_GET_VALUE_NO_EMPTY); \ + _s ? nm_sprintf_bufa (100, "%s", _s) : NULL; \ + }) + /*****************************************************************************/ static NMConfig * @@ -124,7 +135,7 @@ test_config_simple (void) config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "", NULL, "/no/such/dir", "", NULL); g_assert_cmpstr (nm_config_data_get_config_main_file (nm_config_get_data_orig (config)), ==, SRCDIR "/NetworkManager.conf"); - g_assert_cmpstr (nm_config_get_dhcp_client (config), ==, "dhclient"); + g_assert_cmpstr (_config_get_dhcp_client_a (config), ==, "dhclient"); g_assert_cmpstr (nm_config_get_log_level (config), ==, "INFO"); g_assert_cmpint (nm_config_data_get_connectivity_interval (nm_config_get_data_orig (config)), ==, 100); @@ -224,7 +235,7 @@ test_config_override (void) NULL); g_assert_cmpstr (nm_config_data_get_config_main_file (nm_config_get_data_orig (config)), ==, SRCDIR "/NetworkManager.conf"); - g_assert_cmpstr (nm_config_get_dhcp_client (config), ==, "dhclient"); + g_assert_cmpstr (_config_get_dhcp_client_a (config), ==, "dhclient"); g_assert_cmpstr (nm_config_get_log_level (config), ==, "INFO"); g_assert_cmpint (nm_config_data_get_connectivity_interval (nm_config_get_data_orig (config)), ==, 12); @@ -377,7 +388,7 @@ test_config_confdir (void) config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "", NULL, SRCDIR "/conf.d", "", NULL); g_assert_cmpstr (nm_config_data_get_config_main_file (nm_config_get_data_orig (config)), ==, SRCDIR "/NetworkManager.conf"); - g_assert_cmpstr (nm_config_get_dhcp_client (config), ==, "dhcpcd"); + g_assert_cmpstr (_config_get_dhcp_client_a (config), ==, "dhcpcd"); g_assert_cmpstr (nm_config_get_log_level (config), ==, "INFO"); g_assert_cmpstr (nm_config_get_log_domains (config), ==, "PLATFORM,DNS,WIFI"); g_assert_cmpstr (nm_config_data_get_connectivity_uri (nm_config_get_data_orig (config)), ==, "http://example.net");