From 429816080e212b5adb74092b1c3e1e010742ff59 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 24 Jun 2021 15:02:56 +0200 Subject: [PATCH 1/4] hostname: simplify _set_hostname() code - drop nm_hostname_manager_read_hostname() from header file. It's only used internally. - inline some code and drop helper functions. --- src/core/nm-hostname-manager.c | 124 ++++++++++++--------------------- src/core/nm-hostname-manager.h | 2 - 2 files changed, 46 insertions(+), 80 deletions(-) diff --git a/src/core/nm-hostname-manager.c b/src/core/nm-hostname-manager.c index 91e9baaafb..c204dafb63 100644 --- a/src/core/nm-hostname-manager.c +++ b/src/core/nm-hostname-manager.c @@ -162,21 +162,51 @@ hostname_is_dynamic(void) } #endif -/* Returns an allocated string which the caller owns and must eventually free */ -char * -nm_hostname_manager_read_hostname(NMHostnameManager *self) +/*****************************************************************************/ + +const char * +nm_hostname_manager_get_hostname(NMHostnameManager *self) +{ + g_return_val_if_fail(NM_IS_HOSTNAME_MANAGER(self), NULL); + return NM_HOSTNAME_MANAGER_GET_PRIVATE(self)->current_hostname; +} + +static void +_set_hostname(NMHostnameManager *self, const char *hostname) +{ + NMHostnameManagerPrivate *priv = NM_HOSTNAME_MANAGER_GET_PRIVATE(self); + char * old_hostname; + + hostname = nm_str_not_empty(hostname); + + if (nm_streq0(hostname, priv->current_hostname)) + return; + + _LOGI("hostname changed from %s%s%s to %s%s%s", + NM_PRINT_FMT_QUOTED(priv->current_hostname, "\"", priv->current_hostname, "\"", "(none)"), + NM_PRINT_FMT_QUOTED(hostname, "\"", hostname, "\"", "(none)")); + + old_hostname = priv->current_hostname; + priv->current_hostname = g_strdup(hostname); + g_free(old_hostname); + + _notify(self, PROP_HOSTNAME); +} + +static void +_set_hostname_read_file(NMHostnameManager *self) { NMHostnameManagerPrivate *priv = NM_HOSTNAME_MANAGER_GET_PRIVATE(self); - char * hostname = NULL; + gs_free char * hostname = NULL; if (priv->hostnamed_proxy) { - hostname = g_strdup(priv->current_hostname); - goto out; + /* read-hostname returns the current hostname with hostnamed. */ + return; } #if defined(HOSTNAME_PERSIST_SUSE) if (priv->dhcp_monitor_id && hostname_is_dynamic()) - return NULL; + return; #endif #if defined(HOSTNAME_PERSIST_GENTOO) @@ -188,67 +218,7 @@ nm_hostname_manager_read_hostname(NMHostnameManager *self) g_strchomp(hostname); #endif -out: - if (hostname && !hostname[0]) { - g_free(hostname); - return NULL; - } - - return hostname; -} - -/*****************************************************************************/ - -const char * -nm_hostname_manager_get_hostname(NMHostnameManager *self) -{ - g_return_val_if_fail(NM_IS_HOSTNAME_MANAGER(self), NULL); - return NM_HOSTNAME_MANAGER_GET_PRIVATE(self)->current_hostname; -} - -static void -_set_hostname_take(NMHostnameManager *self, char *hostname) -{ - NMHostnameManagerPrivate *priv = NM_HOSTNAME_MANAGER_GET_PRIVATE(self); - - _LOGI("hostname changed from %s%s%s to %s%s%s", - NM_PRINT_FMT_QUOTED(priv->current_hostname, "\"", priv->current_hostname, "\"", "(none)"), - NM_PRINT_FMT_QUOTED(hostname, "\"", hostname, "\"", "(none)")); - - g_free(priv->current_hostname); - priv->current_hostname = hostname; - _notify(self, PROP_HOSTNAME); -} - -static void -_set_hostname(NMHostnameManager *self, const char *hostname) -{ - NMHostnameManagerPrivate *priv = NM_HOSTNAME_MANAGER_GET_PRIVATE(self); - - hostname = nm_str_not_empty(hostname); - if (!nm_streq0(hostname, priv->current_hostname)) - _set_hostname_take(self, g_strdup(hostname)); -} - -static void -_set_hostname_read(NMHostnameManager *self) -{ - NMHostnameManagerPrivate *priv = NM_HOSTNAME_MANAGER_GET_PRIVATE(self); - char * hostname; - - if (priv->hostnamed_proxy) { - /* read-hostname returns the current hostname with hostnamed. */ - return; - } - - hostname = nm_hostname_manager_read_hostname(self); - - if (nm_streq0(hostname, priv->current_hostname)) { - g_free(hostname); - return; - } - - _set_hostname_take(self, hostname); + _set_hostname(self, hostname); } /*****************************************************************************/ @@ -462,7 +432,7 @@ hostname_file_changed_cb(GFileMonitor * monitor, GFileMonitorEvent event_type, gpointer user_data) { - _set_hostname_read(user_data); + _set_hostname_read_file(user_data); } /*****************************************************************************/ @@ -473,15 +443,13 @@ hostnamed_properties_changed(GDBusProxy *proxy, char ** invalidated_properties, gpointer user_data) { - NMHostnameManager * self = user_data; - NMHostnameManagerPrivate *priv = NM_HOSTNAME_MANAGER_GET_PRIVATE(self); - GVariant * v_hostname; + NMHostnameManager * self = user_data; + NMHostnameManagerPrivate *priv = NM_HOSTNAME_MANAGER_GET_PRIVATE(self); + gs_unref_variant GVariant *variant = NULL; - v_hostname = g_dbus_proxy_get_cached_property(priv->hostnamed_proxy, "StaticHostname"); - if (v_hostname) { - _set_hostname(self, g_variant_get_string(v_hostname, NULL)); - g_variant_unref(v_hostname); - } + variant = g_dbus_proxy_get_cached_property(priv->hostnamed_proxy, "StaticHostname"); + if (variant && g_variant_is_of_type(variant, G_VARIANT_TYPE_STRING)) + _set_hostname(self, g_variant_get_string(variant, NULL)); } static void @@ -527,7 +495,7 @@ setup_hostname_file_monitors(NMHostnameManager *self) } #endif - _set_hostname_read(self); + _set_hostname_read_file(self); } /*****************************************************************************/ diff --git a/src/core/nm-hostname-manager.h b/src/core/nm-hostname-manager.h index 0fa137713a..c13167e7f1 100644 --- a/src/core/nm-hostname-manager.h +++ b/src/core/nm-hostname-manager.h @@ -36,8 +36,6 @@ NMHostnameManager *nm_hostname_manager_get(void); const char *nm_hostname_manager_get_hostname(NMHostnameManager *self); -char *nm_hostname_manager_read_hostname(NMHostnameManager *self); - gboolean nm_hostname_manager_write_hostname(NMHostnameManager *self, const char *hostname); void nm_hostname_manager_set_transient_hostname(NMHostnameManager * self, From dbe4803d846638fa65b1a735595341d5a5423cf6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 24 Jun 2021 15:10:08 +0200 Subject: [PATCH 2/4] hostname: use nm_utils_user_data_pack() instead of SetHostnameInfo struct --- src/core/nm-hostname-manager.c | 31 +++++++++++-------------------- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/src/core/nm-hostname-manager.c b/src/core/nm-hostname-manager.c index c204dafb63..e79cb5274d 100644 --- a/src/core/nm-hostname-manager.c +++ b/src/core/nm-hostname-manager.c @@ -223,30 +223,27 @@ _set_hostname_read_file(NMHostnameManager *self) /*****************************************************************************/ -typedef struct { - char * hostname; - NMHostnameManagerSetHostnameCb cb; - gpointer user_data; -} SetHostnameInfo; - static void set_transient_hostname_done(GObject *object, GAsyncResult *res, gpointer user_data) { - GDBusProxy *proxy = G_DBUS_PROXY(object); - gs_free SetHostnameInfo *info = user_data; - gs_unref_variant GVariant *result = NULL; - gs_free_error GError *error = NULL; + GDBusProxy * proxy = G_DBUS_PROXY(object); + gs_unref_variant GVariant *result = NULL; + gs_free_error GError * error = NULL; + gs_free char * hostname = NULL; + NMHostnameManagerSetHostnameCb cb; + gpointer cb_user_data; + + nm_utils_user_data_unpack(user_data, &hostname, &cb, &cb_user_data); result = g_dbus_proxy_call_finish(proxy, res, &error); if (error) { _LOGW("couldn't set the system hostname to '%s' using hostnamed: %s", - info->hostname, + hostname, error->message); } - info->cb(info->hostname, !error, info->user_data); - g_free(info->hostname); + cb(hostname, !error, cb_user_data); } void @@ -256,7 +253,6 @@ nm_hostname_manager_set_transient_hostname(NMHostnameManager * self, gpointer user_data) { NMHostnameManagerPrivate *priv; - SetHostnameInfo * info; g_return_if_fail(NM_IS_HOSTNAME_MANAGER(self)); @@ -267,11 +263,6 @@ nm_hostname_manager_set_transient_hostname(NMHostnameManager * self, return; } - info = g_new0(SetHostnameInfo, 1); - info->hostname = g_strdup(hostname); - info->cb = cb; - info->user_data = user_data; - g_dbus_proxy_call(priv->hostnamed_proxy, "SetHostname", g_variant_new("(sb)", hostname, FALSE), @@ -279,7 +270,7 @@ nm_hostname_manager_set_transient_hostname(NMHostnameManager * self, -1, NULL, set_transient_hostname_done, - info); + nm_utils_user_data_pack(g_strdup(hostname), cb, user_data)); } gboolean From 05aa7519572d0c6985944a617b469e3fb3eb3095 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 24 Jun 2021 15:14:45 +0200 Subject: [PATCH 3/4] core,glib-aux: move nm_hostname_manager_validate_hostname() to shared-utils This function is badly named, because it has no NMHostnameManager self argument. It's just a simple function that entirely operates on a string argument. Move it away from "nm-hostname-manager.h" to "libnm-glib-aux/nm-shared-utils.h". Hostname handling is complicated enough. Simple string validation functions should not obscure the view on the complicated parts. --- src/core/devices/nm-device.c | 2 +- src/core/nm-hostname-manager.c | 27 -------------------------- src/core/nm-hostname-manager.h | 2 -- src/core/settings/nm-settings.c | 2 +- src/libnm-glib-aux/nm-shared-utils.c | 29 ++++++++++++++++++++++++++++ src/libnm-glib-aux/nm-shared-utils.h | 2 ++ 6 files changed, 33 insertions(+), 31 deletions(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index d963863353..1dd998badd 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -17653,7 +17653,7 @@ hostname_dns_lookup_callback(GObject *source, GAsyncResult *result, gpointer use gboolean valid; resolver->hostname = g_steal_pointer(&output); - valid = nm_hostname_manager_validate_hostname(resolver->hostname); + valid = nm_utils_validate_hostname(resolver->hostname); _LOGD(LOGD_DNS, "hostname-from-dns: lookup done for %s, result %s%s%s%s", diff --git a/src/core/nm-hostname-manager.c b/src/core/nm-hostname-manager.c index e79cb5274d..12610c0c0f 100644 --- a/src/core/nm-hostname-manager.c +++ b/src/core/nm-hostname-manager.c @@ -389,33 +389,6 @@ nm_hostname_manager_write_hostname(NMHostnameManager *self, const char *hostname return TRUE; } -gboolean -nm_hostname_manager_validate_hostname(const char *hostname) -{ - const char *p; - gboolean dot = TRUE; - - if (!hostname || !hostname[0]) - return FALSE; - - for (p = hostname; *p; p++) { - if (*p == '.') { - if (dot) - return FALSE; - dot = TRUE; - } else { - if (!g_ascii_isalnum(*p) && (*p != '-') && (*p != '_')) - return FALSE; - dot = FALSE; - } - } - - if (dot) - return FALSE; - - return (p - hostname <= HOST_NAME_MAX); -} - static void hostname_file_changed_cb(GFileMonitor * monitor, GFile * file, diff --git a/src/core/nm-hostname-manager.h b/src/core/nm-hostname-manager.h index c13167e7f1..ff109e7e89 100644 --- a/src/core/nm-hostname-manager.h +++ b/src/core/nm-hostname-manager.h @@ -45,6 +45,4 @@ void nm_hostname_manager_set_transient_hostname(NMHostnameManager * s gboolean nm_hostname_manager_get_transient_hostname(NMHostnameManager *self, char **hostname); -gboolean nm_hostname_manager_validate_hostname(const char *hostname); - #endif /* __NM_HOSTNAME_MANAGER_H__ */ diff --git a/src/core/settings/nm-settings.c b/src/core/settings/nm-settings.c index c876ea148c..ee6ac59314 100644 --- a/src/core/settings/nm-settings.c +++ b/src/core/settings/nm-settings.c @@ -3441,7 +3441,7 @@ impl_settings_save_hostname(NMDBusObject * obj, g_variant_get(parameters, "(&s)", &hostname); /* Minimal validation of the hostname */ - if (!nm_hostname_manager_validate_hostname(hostname)) { + if (!nm_utils_validate_hostname(hostname)) { error_code = NM_SETTINGS_ERROR_INVALID_HOSTNAME; error_reason = "The hostname was too long or contained invalid characters"; goto err; diff --git a/src/libnm-glib-aux/nm-shared-utils.c b/src/libnm-glib-aux/nm-shared-utils.c index 908f6bec8e..68598728cb 100644 --- a/src/libnm-glib-aux/nm-shared-utils.c +++ b/src/libnm-glib-aux/nm-shared-utils.c @@ -6356,3 +6356,32 @@ nm_utils_get_process_exit_status_desc(int status) else return g_strdup_printf("exited with unknown status 0x%x", status); } + +/*****************************************************************************/ + +gboolean +nm_utils_validate_hostname(const char *hostname) +{ + const char *p; + gboolean dot = TRUE; + + if (!hostname || !hostname[0]) + return FALSE; + + for (p = hostname; *p; p++) { + if (*p == '.') { + if (dot) + return FALSE; + dot = TRUE; + } else { + if (!g_ascii_isalnum(*p) && (*p != '-') && (*p != '_')) + return FALSE; + dot = FALSE; + } + } + + if (dot) + return FALSE; + + return (p - hostname <= HOST_NAME_MAX); +} diff --git a/src/libnm-glib-aux/nm-shared-utils.h b/src/libnm-glib-aux/nm-shared-utils.h index 49de9e12b9..803a730c6a 100644 --- a/src/libnm-glib-aux/nm-shared-utils.h +++ b/src/libnm-glib-aux/nm-shared-utils.h @@ -2992,4 +2992,6 @@ void nm_crypto_md5_hash(const guint8 *salt, char *nm_utils_get_process_exit_status_desc(int status); +gboolean nm_utils_validate_hostname(const char *hostname); + #endif /* __NM_SHARED_UTILS_H__ */ From 37b72e8984c906d71793d5e633cb5c53e833d409 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 24 Jun 2021 15:27:05 +0200 Subject: [PATCH 4/4] hostname: cleanup file monitors in NMHostnameManager --- src/core/nm-hostname-manager.c | 91 ++++++++++++++++++++-------------- 1 file changed, 53 insertions(+), 38 deletions(-) diff --git a/src/core/nm-hostname-manager.c b/src/core/nm-hostname-manager.c index 12610c0c0f..6092ed782b 100644 --- a/src/core/nm-hostname-manager.c +++ b/src/core/nm-hostname-manager.c @@ -86,6 +86,19 @@ NM_DEFINE_SINGLETON_GETTER(NMHostnameManager, nm_hostname_manager_get, NM_TYPE_H /*****************************************************************************/ +static inline GFileMonitor * +_file_monitor_new(const char *path) +{ + gs_unref_object GFile *file = NULL; + + nm_assert(path); + + file = g_file_new_for_path(path); + return g_file_monitor_file(file, G_FILE_MONITOR_NONE, NULL, NULL); +} + +/*****************************************************************************/ + #if defined(HOSTNAME_PERSIST_GENTOO) static char * read_hostname_gentoo(const char *path) @@ -389,16 +402,6 @@ nm_hostname_manager_write_hostname(NMHostnameManager *self, const char *hostname return TRUE; } -static void -hostname_file_changed_cb(GFileMonitor * monitor, - GFile * file, - GFile * other_file, - GFileMonitorEvent event_type, - gpointer user_data) -{ - _set_hostname_read_file(user_data); -} - /*****************************************************************************/ static void @@ -416,15 +419,46 @@ hostnamed_properties_changed(GDBusProxy *proxy, _set_hostname(self, g_variant_get_string(variant, NULL)); } +/*****************************************************************************/ + static void -setup_hostname_file_monitors(NMHostnameManager *self) +_file_monitors_file_changed_cb(GFileMonitor * monitor, + GFile * file, + GFile * other_file, + GFileMonitorEvent event_type, + gpointer user_data) +{ + _set_hostname_read_file(user_data); +} + +static void +_file_monitors_clear(NMHostnameManager *self) +{ + NMHostnameManagerPrivate *priv = NM_HOSTNAME_MANAGER_GET_PRIVATE(self); + + if (priv->monitor) { + nm_clear_g_signal_handler(priv->monitor, &priv->monitor_id); + g_file_monitor_cancel(priv->monitor); + g_clear_object(&priv->monitor); + } + + if (priv->dhcp_monitor) { + nm_clear_g_signal_handler(priv->dhcp_monitor, &priv->dhcp_monitor_id); + g_file_monitor_cancel(priv->dhcp_monitor); + g_clear_object(&priv->dhcp_monitor); + } +} + +static void +_file_monitors_setup(NMHostnameManager *self) { NMHostnameManagerPrivate *priv = NM_HOSTNAME_MANAGER_GET_PRIVATE(self); GFileMonitor * monitor; const char * path = HOSTNAME_FILE; - char * link_path = NULL; + gs_free char * link_path = NULL; struct stat file_stat; - GFile * file; + + _file_monitors_clear(self); /* resolve the path to the hostname file if it is a symbolic link */ if (lstat(path, &file_stat) == 0 && S_ISLNK(file_stat.st_mode) @@ -437,24 +471,19 @@ setup_hostname_file_monitors(NMHostnameManager *self) } /* monitor changes to hostname file */ - file = g_file_new_for_path(path); - monitor = g_file_monitor_file(file, G_FILE_MONITOR_NONE, NULL, NULL); - g_object_unref(file); - g_free(link_path); + monitor = _file_monitor_new(path); if (monitor) { priv->monitor_id = - g_signal_connect(monitor, "changed", G_CALLBACK(hostname_file_changed_cb), self); + g_signal_connect(monitor, "changed", G_CALLBACK(_file_monitors_file_changed_cb), self); priv->monitor = monitor; } #if defined(HOSTNAME_PERSIST_SUSE) /* monitor changes to dhcp file to know whether the hostname is valid */ - file = g_file_new_for_path(CONF_DHCP); - monitor = g_file_monitor_file(file, G_FILE_MONITOR_NONE, NULL, NULL); - g_object_unref(file); + monitor = _file_monitor_new(CONF_DHCP); if (monitor) { priv->dhcp_monitor_id = - g_signal_connect(monitor, "changed", G_CALLBACK(hostname_file_changed_cb), self); + g_signal_connect(monitor, "changed", G_CALLBACK(_file_monitors_file_changed_cb), self); priv->dhcp_monitor = monitor; } #endif @@ -523,7 +552,7 @@ constructed(GObject *object) } if (!priv->hostnamed_proxy) - setup_hostname_file_monitors(self); + _file_monitors_setup(self); G_OBJECT_CLASS(nm_hostname_manager_parent_class)->constructed(object); } @@ -541,21 +570,7 @@ dispose(GObject *object) g_clear_object(&priv->hostnamed_proxy); } - if (priv->monitor) { - if (priv->monitor_id) - g_signal_handler_disconnect(priv->monitor, priv->monitor_id); - - g_file_monitor_cancel(priv->monitor); - g_clear_object(&priv->monitor); - } - - if (priv->dhcp_monitor) { - if (priv->dhcp_monitor_id) - g_signal_handler_disconnect(priv->dhcp_monitor, priv->dhcp_monitor_id); - - g_file_monitor_cancel(priv->dhcp_monitor); - g_clear_object(&priv->dhcp_monitor); - } + _file_monitors_clear(self); nm_clear_g_free(&priv->current_hostname);