From 44f68600e15a97b34a7207941b406005d1704b84 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 22 Apr 2017 08:37:32 +0200 Subject: modem: use our standard pattern for accessing private data of NMModeManager Although our type structures have their _priv data embedded, we don't use it directly. Adjust NMModemManager to follow that pattern. --- src/devices/wwan/nm-modem-manager.c | 138 ++++++++++++++++++++++-------------- 1 file changed, 83 insertions(+), 55 deletions(-) diff --git a/src/devices/wwan/nm-modem-manager.c b/src/devices/wwan/nm-modem-manager.c index b1f6d92e7f..edf61d142a 100644 --- a/src/devices/wwan/nm-modem-manager.c +++ b/src/devices/wwan/nm-modem-manager.c @@ -85,16 +85,17 @@ G_DEFINE_TYPE (NMModemManager, nm_modem_manager, G_TYPE_OBJECT) static void handle_new_modem (NMModemManager *self, NMModem *modem) { + NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); const char *path; path = nm_modem_get_path (modem); - if (g_hash_table_lookup (self->_priv.modems, path)) { + if (g_hash_table_lookup (priv->modems, path)) { g_warn_if_reached (); return; } /* Track the new modem */ - g_hash_table_insert (self->_priv.modems, g_strdup (path), modem); + g_hash_table_insert (priv->modems, g_strdup (path), modem); g_signal_emit (self, signals[MODEM_ADDED], 0, modem); } @@ -108,12 +109,14 @@ remove_one_modem (gpointer key, gpointer value, gpointer user_data) static void clear_modem_manager (NMModemManager *self) { - if (!self->_priv.modem_manager) + NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); + + if (!priv->modem_manager) return; - nm_clear_g_signal_handler (self->_priv.modem_manager, &self->_priv.mm_name_owner_changed_id); - nm_clear_g_signal_handler (self->_priv.modem_manager, &self->_priv.mm_object_added_id); - nm_clear_g_signal_handler (self->_priv.modem_manager, &self->_priv.mm_object_removed_id); - g_clear_object (&self->_priv.modem_manager); + nm_clear_g_signal_handler (priv->modem_manager, &priv->mm_name_owner_changed_id); + nm_clear_g_signal_handler (priv->modem_manager, &priv->mm_object_added_id); + nm_clear_g_signal_handler (priv->modem_manager, &priv->mm_object_removed_id); + g_clear_object (&priv->modem_manager); } static void @@ -121,6 +124,7 @@ modem_object_added (MMManager *modem_manager, MMObject *modem_object, NMModemManager *self) { + NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); const gchar *path; MMModem *modem_iface; NMModem *modem; @@ -128,7 +132,7 @@ modem_object_added (MMManager *modem_manager, /* Ensure we don't have the same modem already */ path = mm_object_get_path (modem_object); - if (g_hash_table_lookup (self->_priv.modems, path)) { + if (g_hash_table_lookup (priv->modems, path)) { nm_log_warn (LOGD_MB, "modem with path %s already exists, ignoring", path); return; } @@ -162,29 +166,31 @@ modem_object_removed (MMManager *manager, MMObject *modem_object, NMModemManager *self) { + NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); NMModem *modem; const gchar *path; path = mm_object_get_path (modem_object); - modem = (NMModem *) g_hash_table_lookup (self->_priv.modems, path); + modem = (NMModem *) g_hash_table_lookup (priv->modems, path); if (!modem) return; nm_modem_emit_removed (modem); - g_hash_table_remove (self->_priv.modems, path); + g_hash_table_remove (priv->modems, path); } static void modem_manager_available (NMModemManager *self) { + NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); GList *modems, *l; nm_log_info (LOGD_MB, "ModemManager available in the bus"); /* Update initial modems list */ - modems = g_dbus_object_manager_get_objects (G_DBUS_OBJECT_MANAGER (self->_priv.modem_manager)); + modems = g_dbus_object_manager_get_objects (G_DBUS_OBJECT_MANAGER (priv->modem_manager)); for (l = modems; l; l = g_list_next (l)) - modem_object_added (self->_priv.modem_manager, MM_OBJECT (l->data), self); + modem_object_added (priv->modem_manager, MM_OBJECT (l->data), self); g_list_free_full (modems, (GDestroyNotify) g_object_unref); } @@ -197,10 +203,11 @@ modem_manager_name_owner_changed (MMManager *modem_manager, GParamSpec *pspec, NMModemManager *self) { + NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); gchar *name_owner; /* Quit poking, if any */ - nm_clear_g_source (&self->_priv.mm_launch_id); + nm_clear_g_source (&priv->mm_launch_id); name_owner = g_dbus_object_manager_client_get_name_owner (G_DBUS_OBJECT_MANAGER_CLIENT (modem_manager)); if (!name_owner) { @@ -229,9 +236,11 @@ modem_manager_name_owner_changed (MMManager *modem_manager, } #if WITH_OFONO + static void ofono_create_modem (NMModemManager *self, const char *path) { + NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); NMModem *modem = NULL; /* Ensure duplicate modems aren't created. Because we're not using the @@ -239,7 +248,7 @@ ofono_create_modem (NMModemManager *self, const char *path) * receive ModemAdded signals before GetModems() returns, so some of the * modems returned from GetModems() may already have been created. */ - if (!g_hash_table_lookup (self->_priv.modems, path)) { + if (!g_hash_table_lookup (priv->modems, path)) { modem = nm_modem_ofono_new (path); if (modem) handle_new_modem (self, modem); @@ -256,6 +265,7 @@ ofono_signal_cb (GDBusProxy *proxy, gpointer user_data) { NMModemManager *self = NM_MODEM_MANAGER (user_data); + NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); gchar *object_path; NMModem *modem; @@ -269,10 +279,10 @@ ofono_signal_cb (GDBusProxy *proxy, g_variant_get (parameters, "(o)", &object_path); nm_log_info (LOGD_MB, "oFono modem removed: %s", object_path); - modem = (NMModem *) g_hash_table_lookup (self->_priv.modems, object_path); + modem = (NMModem *) g_hash_table_lookup (priv->modems, object_path); if (modem) { nm_modem_emit_removed (modem); - g_hash_table_remove (self->_priv.modems, object_path); + g_hash_table_remove (priv->modems, object_path); } else { nm_log_warn (LOGD_MB, "could not remove modem %s, not found in table", object_path); @@ -308,13 +318,14 @@ ofono_enumerate_devices_done (GDBusProxy *proxy, GAsyncResult *res, gpointer use static void ofono_check_name_owner (NMModemManager *self) { + NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); gs_free char *name_owner = NULL; - name_owner = g_dbus_proxy_get_name_owner (G_DBUS_PROXY (self->_priv.ofono_proxy)); + name_owner = g_dbus_proxy_get_name_owner (G_DBUS_PROXY (priv->ofono_proxy)); if (name_owner) { nm_log_info (LOGD_MB, "oFono is now available"); - g_dbus_proxy_call (self->_priv.ofono_proxy, + g_dbus_proxy_call (priv->ofono_proxy, "GetModems", NULL, G_DBUS_CALL_FLAGS_NONE, @@ -329,7 +340,7 @@ ofono_check_name_owner (NMModemManager *self) nm_log_info (LOGD_MB, "oFono disappeared from bus"); /* Remove any oFono modems that might be left around */ - g_hash_table_iter_init (&iter, self->_priv.modems); + g_hash_table_iter_init (&iter, priv->modems); while (g_hash_table_iter_next (&iter, NULL, (gpointer) &modem)) { if (NM_IS_MODEM_OFONO (modem)) { nm_modem_emit_removed (modem); @@ -351,20 +362,21 @@ static void ofono_proxy_new_cb (GObject *source_object, GAsyncResult *res, gpointer user_data) { gs_unref_object NMModemManager *self = NM_MODEM_MANAGER (user_data); + NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); gs_free_error GError *error = NULL; - self->_priv.ofono_proxy = g_dbus_proxy_new_finish (res, &error); + priv->ofono_proxy = g_dbus_proxy_new_finish (res, &error); if (error) { nm_log_warn (LOGD_MB, "error getting oFono bus proxy: %s", error->message); return; } - g_signal_connect (self->_priv.ofono_proxy, + g_signal_connect (priv->ofono_proxy, "notify::g-name-owner", G_CALLBACK (ofono_name_owner_changed), self); - g_signal_connect (self->_priv.ofono_proxy, + g_signal_connect (priv->ofono_proxy, "g-signal", G_CALLBACK (ofono_signal_cb), self); @@ -375,8 +387,10 @@ ofono_proxy_new_cb (GObject *source_object, GAsyncResult *res, gpointer user_dat static void ensure_ofono_client (NMModemManager *self) { - g_assert (self->_priv.dbus_connection); - g_dbus_proxy_new (self->_priv.dbus_connection, + NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); + + g_assert (priv->dbus_connection); + g_dbus_proxy_new (priv->dbus_connection, G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START, NULL, OFONO_DBUS_SERVICE, @@ -420,8 +434,10 @@ modem_manager_poke_cb (GDBusConnection *connection, static void modem_manager_poke (NMModemManager *self) { + NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); + /* If there is no current owner right away, ensure we poke to get one */ - g_dbus_connection_call (self->_priv.dbus_connection, + g_dbus_connection_call (priv->dbus_connection, "org.freedesktop.ModemManager1", "/org/freedesktop/ModemManager1", DBUS_INTERFACE_PEER, @@ -438,9 +454,10 @@ modem_manager_poke (NMModemManager *self) static void modem_manager_check_name_owner (NMModemManager *self) { + NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); gs_free gchar *name_owner = NULL; - name_owner = g_dbus_object_manager_client_get_name_owner (G_DBUS_OBJECT_MANAGER_CLIENT (self->_priv.modem_manager)); + name_owner = g_dbus_object_manager_client_get_name_owner (G_DBUS_OBJECT_MANAGER_CLIENT (priv->modem_manager)); if (name_owner) { /* Available! */ modem_manager_available (self); @@ -457,14 +474,15 @@ manager_new_ready (GObject *source, GAsyncResult *res, NMModemManager *self) { - /* Note we always get an extra reference to self here */ - + NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); GError *error = NULL; - g_return_if_fail (!self->_priv.modem_manager); + /* Note we always get an extra reference to self here */ + + g_return_if_fail (!priv->modem_manager); - self->_priv.modem_manager = mm_manager_new_finish (res, &error); - if (!self->_priv.modem_manager) { + priv->modem_manager = mm_manager_new_finish (res, &error); + if (!priv->modem_manager) { /* We're not really supposed to get any error here. If we do get one, * though, just re-schedule the MMManager creation after some time. * During this period, name-owner changes won't be followed. */ @@ -474,18 +492,18 @@ manager_new_ready (GObject *source, schedule_modem_manager_relaunch (self, MODEM_POKE_INTERVAL); } else { /* Setup signals in the GDBusObjectManagerClient */ - self->_priv.mm_name_owner_changed_id = - g_signal_connect (self->_priv.modem_manager, + priv->mm_name_owner_changed_id = + g_signal_connect (priv->modem_manager, "notify::name-owner", G_CALLBACK (modem_manager_name_owner_changed), self); - self->_priv.mm_object_added_id = - g_signal_connect (self->_priv.modem_manager, + priv->mm_object_added_id = + g_signal_connect (priv->modem_manager, "object-added", G_CALLBACK (modem_object_added), self); - self->_priv.mm_object_removed_id = - g_signal_connect (self->_priv.modem_manager, + priv->mm_object_removed_id = + g_signal_connect (priv->modem_manager, "object-removed", G_CALLBACK (modem_object_removed), self); @@ -500,13 +518,15 @@ manager_new_ready (GObject *source, static void ensure_modem_manager (NMModemManager *self) { - g_assert (self->_priv.dbus_connection); + NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); + + g_assert (priv->dbus_connection); /* Create the GDBusObjectManagerClient. We do not request to autostart, as * we don't really want the MMManager creation to fail. We can always poke * later on if we want to request the autostart */ - if (!self->_priv.modem_manager) { - mm_manager_new (self->_priv.dbus_connection, + if (!priv->modem_manager) { + mm_manager_new (priv->dbus_connection, G_DBUS_OBJECT_MANAGER_CLIENT_FLAGS_DO_NOT_AUTO_START, NULL, (GAsyncReadyCallback)manager_new_ready, @@ -521,7 +541,9 @@ ensure_modem_manager (NMModemManager *self) static gboolean mm_launch_cb (NMModemManager *self) { - self->_priv.mm_launch_id = 0; + NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); + + priv->mm_launch_id = 0; ensure_modem_manager (self); return G_SOURCE_REMOVE; } @@ -530,12 +552,14 @@ static void schedule_modem_manager_relaunch (NMModemManager *self, guint n_seconds) { + NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); + /* No need to pass an extra reference to self; timeout/idle will be * cancelled if the object gets disposed. */ if (n_seconds) - self->_priv.mm_launch_id = g_timeout_add_seconds (n_seconds, (GSourceFunc)mm_launch_cb, self); + priv->mm_launch_id = g_timeout_add_seconds (n_seconds, (GSourceFunc)mm_launch_cb, self); else - self->_priv.mm_launch_id = g_idle_add ((GSourceFunc)mm_launch_cb, self); + priv->mm_launch_id = g_idle_add ((GSourceFunc)mm_launch_cb, self); } static void @@ -544,10 +568,11 @@ bus_get_ready (GObject *source, gpointer user_data) { gs_unref_object NMModemManager *self = NM_MODEM_MANAGER (user_data); + NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); gs_free_error GError *error = NULL; - self->_priv.dbus_connection = g_bus_get_finish (res, &error); - if (!self->_priv.dbus_connection) { + priv->dbus_connection = g_bus_get_finish (res, &error); + if (!priv->dbus_connection) { nm_log_warn (LOGD_MB, "error getting bus connection: %s", error->message); return; } @@ -564,7 +589,9 @@ bus_get_ready (GObject *source, static void nm_modem_manager_init (NMModemManager *self) { - self->_priv.modems = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_object_unref); + NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); + + priv->modems = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_object_unref); g_bus_get (G_BUS_TYPE_SYSTEM, NULL, @@ -576,24 +603,25 @@ static void dispose (GObject *object) { NMModemManager *self = NM_MODEM_MANAGER (object); + NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); - nm_clear_g_source (&self->_priv.mm_launch_id); + nm_clear_g_source (&priv->mm_launch_id); clear_modem_manager (self); #if WITH_OFONO - if (self->_priv.ofono_proxy) { - g_signal_handlers_disconnect_by_func (self->_priv.ofono_proxy, ofono_name_owner_changed, self); - g_signal_handlers_disconnect_by_func (self->_priv.ofono_proxy, ofono_signal_cb, self); - g_clear_object (&self->_priv.ofono_proxy); + if (priv->ofono_proxy) { + g_signal_handlers_disconnect_by_func (priv->ofono_proxy, ofono_name_owner_changed, self); + g_signal_handlers_disconnect_by_func (priv->ofono_proxy, ofono_signal_cb, self); + g_clear_object (&priv->ofono_proxy); } #endif - g_clear_object (&self->_priv.dbus_connection); + g_clear_object (&priv->dbus_connection); - if (self->_priv.modems) { - g_hash_table_foreach_remove (self->_priv.modems, remove_one_modem, object); - g_hash_table_destroy (self->_priv.modems); + if (priv->modems) { + g_hash_table_foreach_remove (priv->modems, remove_one_modem, object); + g_hash_table_destroy (priv->modems); } G_OBJECT_CLASS (nm_modem_manager_parent_class)->dispose (object); -- cgit v1.2.1 From 6334121d6ba511a0d51bb69ff701fa402c04d265 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 22 Apr 2017 08:47:25 +0200 Subject: modem: make use of cleanup attribute to free data in callbacks --- src/devices/wwan/nm-modem-manager.c | 76 +++++++++++++++++-------------------- 1 file changed, 34 insertions(+), 42 deletions(-) diff --git a/src/devices/wwan/nm-modem-manager.c b/src/devices/wwan/nm-modem-manager.c index edf61d142a..40e469f17e 100644 --- a/src/devices/wwan/nm-modem-manager.c +++ b/src/devices/wwan/nm-modem-manager.c @@ -403,32 +403,27 @@ ensure_ofono_client (NMModemManager *self) #endif static void -modem_manager_poke_cb (GDBusConnection *connection, +modem_manager_poke_cb (GObject *connection, GAsyncResult *res, - NMModemManager *self) + gpointer user_data) { - GError *error = NULL; - GVariant *result; + gs_free_error GError *error = NULL; + gs_unref_variant GVariant *result = NULL; + gs_unref_object NMModemManager *self = user_data; - result = g_dbus_connection_call_finish (connection, res, &error); + result = g_dbus_connection_call_finish (G_DBUS_CONNECTION (connection), res, &error); if (error) { nm_log_warn (LOGD_MB, "error poking ModemManager: %s", - error ? error->message : ""); + error->message); /* Don't reschedule poke is MM service doesn't exist. */ - if (!g_error_matches (error, G_DBUS_ERROR, G_DBUS_ERROR_SERVICE_UNKNOWN) + if ( !g_error_matches (error, G_DBUS_ERROR, G_DBUS_ERROR_SERVICE_UNKNOWN) && !g_error_matches (error, G_DBUS_ERROR, G_DBUS_ERROR_SPAWN_SERVICE_NOT_FOUND)) { /* Setup timeout to relaunch */ schedule_modem_manager_relaunch (self, MODEM_POKE_INTERVAL); } - - g_error_free (error); - } else - g_variant_unref (result); - - /* Balance refcount */ - g_object_unref (self); + } } static void @@ -447,7 +442,7 @@ modem_manager_poke (NMModemManager *self) G_DBUS_CALL_FLAGS_NONE, -1, NULL, /* cancellable */ - (GAsyncReadyCallback)modem_manager_poke_cb, /* callback */ + modem_manager_poke_cb, /* callback */ g_object_ref (self)); /* user_data */ } @@ -472,12 +467,11 @@ modem_manager_check_name_owner (NMModemManager *self) static void manager_new_ready (GObject *source, GAsyncResult *res, - NMModemManager *self) + gpointer user_data) { + gs_unref_object NMModemManager *self = user_data; NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); - GError *error = NULL; - - /* Note we always get an extra reference to self here */ + gs_free_error GError *error = NULL; g_return_if_fail (!priv->modem_manager); @@ -487,32 +481,29 @@ manager_new_ready (GObject *source, * though, just re-schedule the MMManager creation after some time. * During this period, name-owner changes won't be followed. */ nm_log_warn (LOGD_MB, "error creating ModemManager client: %s", error->message); - g_error_free (error); /* Setup timeout to relaunch */ schedule_modem_manager_relaunch (self, MODEM_POKE_INTERVAL); - } else { - /* Setup signals in the GDBusObjectManagerClient */ - priv->mm_name_owner_changed_id = - g_signal_connect (priv->modem_manager, - "notify::name-owner", - G_CALLBACK (modem_manager_name_owner_changed), - self); - priv->mm_object_added_id = - g_signal_connect (priv->modem_manager, - "object-added", - G_CALLBACK (modem_object_added), - self); - priv->mm_object_removed_id = - g_signal_connect (priv->modem_manager, - "object-removed", - G_CALLBACK (modem_object_removed), - self); - - modem_manager_check_name_owner (self); + return; } - /* Balance refcount */ - g_object_unref (self); + /* Setup signals in the GDBusObjectManagerClient */ + priv->mm_name_owner_changed_id = + g_signal_connect (priv->modem_manager, + "notify::name-owner", + G_CALLBACK (modem_manager_name_owner_changed), + self); + priv->mm_object_added_id = + g_signal_connect (priv->modem_manager, + "object-added", + G_CALLBACK (modem_object_added), + self); + priv->mm_object_removed_id = + g_signal_connect (priv->modem_manager, + "object-removed", + G_CALLBACK (modem_object_removed), + self); + + modem_manager_check_name_owner (self); } static void @@ -529,7 +520,7 @@ ensure_modem_manager (NMModemManager *self) mm_manager_new (priv->dbus_connection, G_DBUS_OBJECT_MANAGER_CLIENT_FLAGS_DO_NOT_AUTO_START, NULL, - (GAsyncReadyCallback)manager_new_ready, + manager_new_ready, g_object_ref (self)); return; } @@ -622,6 +613,7 @@ dispose (GObject *object) if (priv->modems) { g_hash_table_foreach_remove (priv->modems, remove_one_modem, object); g_hash_table_destroy (priv->modems); + priv->modems = NULL; } G_OBJECT_CLASS (nm_modem_manager_parent_class)->dispose (object); -- cgit v1.2.1 From 0adc51740714fe648323b5eef8fe336b9a89302b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 22 Apr 2017 09:06:26 +0200 Subject: modem: use logging macros in nm-modem-manager.c --- src/devices/wwan/nm-modem-manager.c | 63 +++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 31 deletions(-) diff --git a/src/devices/wwan/nm-modem-manager.c b/src/devices/wwan/nm-modem-manager.c index 40e469f17e..ca30094508 100644 --- a/src/devices/wwan/nm-modem-manager.c +++ b/src/devices/wwan/nm-modem-manager.c @@ -82,6 +82,11 @@ G_DEFINE_TYPE (NMModemManager, nm_modem_manager, G_TYPE_OBJECT) /*****************************************************************************/ +#define _NMLOG_DOMAIN LOGD_MB +#define _NMLOG(level, ...) __NMLOG_DEFAULT (level, _NMLOG_DOMAIN, "modem-manager", __VA_ARGS__) + +/*****************************************************************************/ + static void handle_new_modem (NMModemManager *self, NMModem *modem) { @@ -133,20 +138,20 @@ modem_object_added (MMManager *modem_manager, /* Ensure we don't have the same modem already */ path = mm_object_get_path (modem_object); if (g_hash_table_lookup (priv->modems, path)) { - nm_log_warn (LOGD_MB, "modem with path %s already exists, ignoring", path); + _LOGW ("modem with path %s already exists, ignoring", path); return; } /* Ensure we have the 'Modem' interface at least */ modem_iface = mm_object_peek_modem (modem_object); if (!modem_iface) { - nm_log_warn (LOGD_MB, "modem with path %s doesn't have the Modem interface, ignoring", path); + _LOGW ("modem with path %s doesn't have the Modem interface, ignoring", path); return; } /* Ensure we have a primary port reported */ if (!mm_modem_get_primary_port (modem_iface)) { - nm_log_warn (LOGD_MB, "modem with path %s has unknown primary port, ignoring", path); + _LOGW ("modem with path %s has unknown primary port, ignoring", path); return; } @@ -154,10 +159,8 @@ modem_object_added (MMManager *modem_manager, modem = nm_modem_broadband_new (G_OBJECT (modem_object), &error); if (modem) handle_new_modem (self, modem); - else { - nm_log_warn (LOGD_MB, "failed to create modem: %s", - error->message); - } + else + _LOGW ("failed to create modem: %s", error->message); g_clear_error (&error); } @@ -185,7 +188,7 @@ modem_manager_available (NMModemManager *self) NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); GList *modems, *l; - nm_log_info (LOGD_MB, "ModemManager available in the bus"); + _LOGI ("ModemManager available in the bus"); /* Update initial modems list */ modems = g_dbus_object_manager_get_objects (G_DBUS_OBJECT_MANAGER (priv->modem_manager)); @@ -211,7 +214,7 @@ modem_manager_name_owner_changed (MMManager *modem_manager, name_owner = g_dbus_object_manager_client_get_name_owner (G_DBUS_OBJECT_MANAGER_CLIENT (modem_manager)); if (!name_owner) { - nm_log_info (LOGD_MB, "ModemManager disappeared from bus"); + _LOGI ("ModemManager disappeared from bus"); /* If not managed by systemd, schedule relaunch */ if (!sd_booted ()) @@ -253,7 +256,7 @@ ofono_create_modem (NMModemManager *self, const char *path) if (modem) handle_new_modem (self, modem); else - nm_log_warn (LOGD_MB, "Failed to create oFono modem for %s", path); + _LOGW ("Failed to create oFono modem for %s", path); } } @@ -271,21 +274,21 @@ ofono_signal_cb (GDBusProxy *proxy, if (g_strcmp0 (signal_name, "ModemAdded") == 0) { g_variant_get (parameters, "(oa{sv})", &object_path, NULL); - nm_log_info (LOGD_MB, "oFono modem appeared: %s", object_path); + _LOGI ("oFono modem appeared: %s", object_path); ofono_create_modem (NM_MODEM_MANAGER (user_data), object_path); g_free (object_path); } else if (g_strcmp0 (signal_name, "ModemRemoved") == 0) { g_variant_get (parameters, "(o)", &object_path); - nm_log_info (LOGD_MB, "oFono modem removed: %s", object_path); + _LOGI ("oFono modem removed: %s", object_path); modem = (NMModem *) g_hash_table_lookup (priv->modems, object_path); if (modem) { nm_modem_emit_removed (modem); g_hash_table_remove (priv->modems, object_path); } else { - nm_log_warn (LOGD_MB, "could not remove modem %s, not found in table", - object_path); + _LOGW ("could not remove modem %s, not found in table", + object_path); } g_free (object_path); } @@ -301,18 +304,17 @@ ofono_enumerate_devices_done (GDBusProxy *proxy, GAsyncResult *res, gpointer use const char *path; results = g_dbus_proxy_call_finish (proxy, res, &error); - if (results) { - g_variant_get (results, "(a(oa{sv}))", &iter); - while (g_variant_iter_loop (iter, "(&oa{sv})", &path, NULL)) - ofono_create_modem (manager, path); - g_variant_iter_free (iter); - g_variant_unref (results); + if (!results) { + _LOGW ("failed to enumerate oFono devices: %s", + error->message); + return; } - if (error) { - nm_log_warn (LOGD_MB, "failed to enumerate oFono devices: %s", - error->message); - } + g_variant_get (results, "(a(oa{sv}))", &iter); + while (g_variant_iter_loop (iter, "(&oa{sv})", &path, NULL)) + ofono_create_modem (manager, path); + g_variant_iter_free (iter); + g_variant_unref (results); } static void @@ -323,7 +325,7 @@ ofono_check_name_owner (NMModemManager *self) name_owner = g_dbus_proxy_get_name_owner (G_DBUS_PROXY (priv->ofono_proxy)); if (name_owner) { - nm_log_info (LOGD_MB, "oFono is now available"); + _LOGI ("oFono is now available"); g_dbus_proxy_call (priv->ofono_proxy, "GetModems", @@ -337,7 +339,7 @@ ofono_check_name_owner (NMModemManager *self) GHashTableIter iter; NMModem *modem; - nm_log_info (LOGD_MB, "oFono disappeared from bus"); + _LOGI ("oFono disappeared from bus"); /* Remove any oFono modems that might be left around */ g_hash_table_iter_init (&iter, priv->modems); @@ -367,7 +369,7 @@ ofono_proxy_new_cb (GObject *source_object, GAsyncResult *res, gpointer user_dat priv->ofono_proxy = g_dbus_proxy_new_finish (res, &error); if (error) { - nm_log_warn (LOGD_MB, "error getting oFono bus proxy: %s", error->message); + _LOGW ("error getting oFono bus proxy: %s", error->message); return; } @@ -413,8 +415,7 @@ modem_manager_poke_cb (GObject *connection, result = g_dbus_connection_call_finish (G_DBUS_CONNECTION (connection), res, &error); if (error) { - nm_log_warn (LOGD_MB, "error poking ModemManager: %s", - error->message); + _LOGW ("error poking ModemManager: %s", error->message); /* Don't reschedule poke is MM service doesn't exist. */ if ( !g_error_matches (error, G_DBUS_ERROR, G_DBUS_ERROR_SERVICE_UNKNOWN) @@ -480,7 +481,7 @@ manager_new_ready (GObject *source, /* We're not really supposed to get any error here. If we do get one, * though, just re-schedule the MMManager creation after some time. * During this period, name-owner changes won't be followed. */ - nm_log_warn (LOGD_MB, "error creating ModemManager client: %s", error->message); + _LOGW ("error creating ModemManager client: %s", error->message); /* Setup timeout to relaunch */ schedule_modem_manager_relaunch (self, MODEM_POKE_INTERVAL); return; @@ -564,7 +565,7 @@ bus_get_ready (GObject *source, priv->dbus_connection = g_bus_get_finish (res, &error); if (!priv->dbus_connection) { - nm_log_warn (LOGD_MB, "error getting bus connection: %s", error->message); + _LOGW ("error getting bus connection: %s", error->message); return; } -- cgit v1.2.1 From 3128a8a4c1a4d697e73daed17e0073953cb80124 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 22 Apr 2017 09:32:36 +0200 Subject: manager: make asynchrounous operations cancellable It is often wrong to take a reference to keep the instance alive during the asynchronous request, because it means the instance cannot be destroyed as long as the (non cancellable) request is bending. Fix that for NMModemManager and pass a cancallable along. --- src/devices/wwan/nm-modem-manager.c | 149 ++++++++++++++++++++++++++++-------- 1 file changed, 116 insertions(+), 33 deletions(-) diff --git a/src/devices/wwan/nm-modem-manager.c b/src/devices/wwan/nm-modem-manager.c index ca30094508..026955ca9f 100644 --- a/src/devices/wwan/nm-modem-manager.c +++ b/src/devices/wwan/nm-modem-manager.c @@ -53,6 +53,11 @@ enum { static guint signals[LAST_SIGNAL] = { 0 }; typedef struct { + /* used during g_bus_get() and later during mm_manager_new(). */ + GCancellable *main_cancellable; + + GCancellable *poke_cancellable; + GDBusConnection *dbus_connection; MMManager *modem_manager; guint mm_launch_id; @@ -62,6 +67,7 @@ typedef struct { #if WITH_OFONO GDBusProxy *ofono_proxy; + GCancellable *ofono_cancellable; #endif GHashTable *modems; @@ -295,15 +301,27 @@ ofono_signal_cb (GDBusProxy *proxy, } static void -ofono_enumerate_devices_done (GDBusProxy *proxy, GAsyncResult *res, gpointer user_data) +ofono_enumerate_devices_done (GObject *proxy, + GAsyncResult *res, + gpointer user_data) { - NMModemManager *manager = NM_MODEM_MANAGER (user_data); + NMModemManager *self; + NMModemManagerPrivate *priv; gs_free_error GError *error = NULL; GVariant *results; GVariantIter *iter; const char *path; - results = g_dbus_proxy_call_finish (proxy, res, &error); + results = g_dbus_proxy_call_finish (G_DBUS_PROXY (proxy), res, &error); + if ( !results + && g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + return; + + self = NM_MODEM_MANAGER (user_data); + priv = NM_MODEM_MANAGER_GET_PRIVATE (self); + + g_clear_object (&priv->ofono_cancellable); + if (!results) { _LOGW ("failed to enumerate oFono devices: %s", error->message); @@ -312,7 +330,7 @@ ofono_enumerate_devices_done (GDBusProxy *proxy, GAsyncResult *res, gpointer use g_variant_get (results, "(a(oa{sv}))", &iter); while (g_variant_iter_loop (iter, "(&oa{sv})", &path, NULL)) - ofono_create_modem (manager, path); + ofono_create_modem (self, path); g_variant_iter_free (iter); g_variant_unref (results); } @@ -327,14 +345,17 @@ ofono_check_name_owner (NMModemManager *self) if (name_owner) { _LOGI ("oFono is now available"); + nm_clear_g_cancellable (&priv->ofono_cancellable); + priv->ofono_cancellable = g_cancellable_new (); + g_dbus_proxy_call (priv->ofono_proxy, "GetModems", NULL, G_DBUS_CALL_FLAGS_NONE, -1, - NULL, - (GAsyncReadyCallback) ofono_enumerate_devices_done, - g_object_ref (self)); + priv->ofono_cancellable, + ofono_enumerate_devices_done, + self); } else { GHashTableIter iter; NMModem *modem; @@ -361,18 +382,32 @@ ofono_name_owner_changed (GDBusProxy *ofono_proxy, } static void -ofono_proxy_new_cb (GObject *source_object, GAsyncResult *res, gpointer user_data) +ofono_proxy_new_cb (GObject *source_object, + GAsyncResult *res, + gpointer user_data) { - gs_unref_object NMModemManager *self = NM_MODEM_MANAGER (user_data); - NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); + NMModemManager *self; + NMModemManagerPrivate *priv; gs_free_error GError *error = NULL; + GDBusProxy *proxy; - priv->ofono_proxy = g_dbus_proxy_new_finish (res, &error); - if (error) { + proxy = g_dbus_proxy_new_finish (res, &error); + if ( !proxy + && g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + return; + + self = NM_MODEM_MANAGER (user_data); + priv = NM_MODEM_MANAGER_GET_PRIVATE (self); + + g_clear_object (&priv->ofono_cancellable); + + if (!proxy) { _LOGW ("error getting oFono bus proxy: %s", error->message); return; } + priv->ofono_proxy = proxy; + g_signal_connect (priv->ofono_proxy, "notify::g-name-owner", G_CALLBACK (ofono_name_owner_changed), @@ -391,16 +426,20 @@ ensure_ofono_client (NMModemManager *self) { NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); - g_assert (priv->dbus_connection); + nm_assert (priv->dbus_connection); + nm_assert (!priv->ofono_cancellable); + + priv->ofono_cancellable = g_cancellable_new (); + g_dbus_proxy_new (priv->dbus_connection, G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START, NULL, OFONO_DBUS_SERVICE, OFONO_DBUS_PATH, OFONO_DBUS_INTERFACE, - NULL, - (GAsyncReadyCallback) ofono_proxy_new_cb, - g_object_ref (self)); + priv->ofono_cancellable, + ofono_proxy_new_cb, + self); } #endif @@ -409,11 +448,22 @@ modem_manager_poke_cb (GObject *connection, GAsyncResult *res, gpointer user_data) { + NMModemManager *self; + NMModemManagerPrivate *priv; gs_free_error GError *error = NULL; gs_unref_variant GVariant *result = NULL; - gs_unref_object NMModemManager *self = user_data; result = g_dbus_connection_call_finish (G_DBUS_CONNECTION (connection), res, &error); + + if ( !result + && g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + return; + + self = user_data; + priv = NM_MODEM_MANAGER_GET_PRIVATE (self); + + g_clear_object (&priv->poke_cancellable); + if (error) { _LOGW ("error poking ModemManager: %s", error->message); @@ -432,6 +482,9 @@ modem_manager_poke (NMModemManager *self) { NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); + nm_clear_g_cancellable (&priv->poke_cancellable); + priv->poke_cancellable = g_cancellable_new (); + /* If there is no current owner right away, ensure we poke to get one */ g_dbus_connection_call (priv->dbus_connection, "org.freedesktop.ModemManager1", @@ -442,9 +495,9 @@ modem_manager_poke (NMModemManager *self) NULL, /* outputs */ G_DBUS_CALL_FLAGS_NONE, -1, - NULL, /* cancellable */ + priv->poke_cancellable, modem_manager_poke_cb, /* callback */ - g_object_ref (self)); /* user_data */ + self); /* user_data */ } static void @@ -470,14 +523,24 @@ manager_new_ready (GObject *source, GAsyncResult *res, gpointer user_data) { - gs_unref_object NMModemManager *self = user_data; - NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); + NMModemManager *self; + NMModemManagerPrivate *priv; gs_free_error GError *error = NULL; + MMManager *modem_manager; - g_return_if_fail (!priv->modem_manager); + modem_manager = mm_manager_new_finish (res, &error); + if ( !modem_manager + && g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + return; - priv->modem_manager = mm_manager_new_finish (res, &error); - if (!priv->modem_manager) { + self = user_data; + priv = NM_MODEM_MANAGER_GET_PRIVATE (self); + + nm_assert (!priv->modem_manager); + + g_clear_object (&priv->main_cancellable); + + if (!modem_manager) { /* We're not really supposed to get any error here. If we do get one, * though, just re-schedule the MMManager creation after some time. * During this period, name-owner changes won't be followed. */ @@ -487,6 +550,8 @@ manager_new_ready (GObject *source, return; } + priv->modem_manager = modem_manager; + /* Setup signals in the GDBusObjectManagerClient */ priv->mm_name_owner_changed_id = g_signal_connect (priv->modem_manager, @@ -518,11 +583,13 @@ ensure_modem_manager (NMModemManager *self) * we don't really want the MMManager creation to fail. We can always poke * later on if we want to request the autostart */ if (!priv->modem_manager) { + if (!priv->main_cancellable) + priv->main_cancellable = g_cancellable_new (); mm_manager_new (priv->dbus_connection, G_DBUS_OBJECT_MANAGER_CLIENT_FLAGS_DO_NOT_AUTO_START, - NULL, + priv->main_cancellable, manager_new_ready, - g_object_ref (self)); + self); return; } @@ -559,16 +626,26 @@ bus_get_ready (GObject *source, GAsyncResult *res, gpointer user_data) { - gs_unref_object NMModemManager *self = NM_MODEM_MANAGER (user_data); - NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); + NMModemManager *self; + NMModemManagerPrivate *priv; gs_free_error GError *error = NULL; + GDBusConnection *connection; + + connection = g_bus_get_finish (res, &error); + if ( !connection + && g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + return; + + self = NM_MODEM_MANAGER (user_data); + priv = NM_MODEM_MANAGER_GET_PRIVATE (self); - priv->dbus_connection = g_bus_get_finish (res, &error); - if (!priv->dbus_connection) { + if (!connection) { _LOGW ("error getting bus connection: %s", error->message); return; } + priv->dbus_connection = connection; + /* Got the bus, ensure clients */ ensure_modem_manager (self); #if WITH_OFONO @@ -585,10 +662,12 @@ nm_modem_manager_init (NMModemManager *self) priv->modems = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_object_unref); + priv->main_cancellable = g_cancellable_new (); + g_bus_get (G_BUS_TYPE_SYSTEM, - NULL, - (GAsyncReadyCallback)bus_get_ready, - g_object_ref (self)); + priv->main_cancellable, + bus_get_ready, + self); } static void @@ -597,6 +676,9 @@ dispose (GObject *object) NMModemManager *self = NM_MODEM_MANAGER (object); NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); + nm_clear_g_cancellable (&priv->main_cancellable); + nm_clear_g_cancellable (&priv->poke_cancellable); + nm_clear_g_source (&priv->mm_launch_id); clear_modem_manager (self); @@ -607,6 +689,7 @@ dispose (GObject *object) g_signal_handlers_disconnect_by_func (priv->ofono_proxy, ofono_signal_cb, self); g_clear_object (&priv->ofono_proxy); } + nm_clear_g_cancellable (&priv->ofono_cancellable); #endif g_clear_object (&priv->dbus_connection); -- cgit v1.2.1 From 4c070639d3a5fbe85bce0c5e8ce91163e1753ccd Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 22 Apr 2017 09:47:23 +0200 Subject: modem/trivial: rename functions in nm-modem-manager.c Most of the functions are strictly related to ModemManager. Their name should hint to that, so that they are clearly separated from the ofono functions and general purpose functions. Same for data fields. --- src/devices/wwan/nm-modem-manager.c | 226 +++++++++++++++++++----------------- 1 file changed, 119 insertions(+), 107 deletions(-) diff --git a/src/devices/wwan/nm-modem-manager.c b/src/devices/wwan/nm-modem-manager.c index 026955ca9f..94a39d6493 100644 --- a/src/devices/wwan/nm-modem-manager.c +++ b/src/devices/wwan/nm-modem-manager.c @@ -53,21 +53,25 @@ enum { static guint signals[LAST_SIGNAL] = { 0 }; typedef struct { + GDBusConnection *dbus_connection; + /* used during g_bus_get() and later during mm_manager_new(). */ GCancellable *main_cancellable; - GCancellable *poke_cancellable; - - GDBusConnection *dbus_connection; - MMManager *modem_manager; - guint mm_launch_id; - gulong mm_name_owner_changed_id; - gulong mm_object_added_id; - gulong mm_object_removed_id; + struct { + MMManager *manager; + GCancellable *poke_cancellable; + gulong handle_name_owner_changed_id; + gulong handle_object_added_id; + gulong handle_object_removed_id; + guint relaunch_id; + } modm; #if WITH_OFONO - GDBusProxy *ofono_proxy; - GCancellable *ofono_cancellable; + struct { + GDBusProxy *proxy; + GCancellable *cancellable; + } ofono; #endif GHashTable *modems; @@ -93,6 +97,12 @@ G_DEFINE_TYPE (NMModemManager, nm_modem_manager, G_TYPE_OBJECT) /*****************************************************************************/ +static void modm_schedule_manager_relaunch (NMModemManager *self, + guint n_seconds); +static void modm_ensure_manager (NMModemManager *self); + +/*****************************************************************************/ + static void handle_new_modem (NMModemManager *self, NMModem *modem) { @@ -117,23 +127,25 @@ remove_one_modem (gpointer key, gpointer value, gpointer user_data) return TRUE; } +/*****************************************************************************/ + static void -clear_modem_manager (NMModemManager *self) +modm_clear_manager (NMModemManager *self) { NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); - if (!priv->modem_manager) + if (!priv->modm.manager) return; - nm_clear_g_signal_handler (priv->modem_manager, &priv->mm_name_owner_changed_id); - nm_clear_g_signal_handler (priv->modem_manager, &priv->mm_object_added_id); - nm_clear_g_signal_handler (priv->modem_manager, &priv->mm_object_removed_id); - g_clear_object (&priv->modem_manager); + nm_clear_g_signal_handler (priv->modm.manager, &priv->modm.handle_name_owner_changed_id); + nm_clear_g_signal_handler (priv->modm.manager, &priv->modm.handle_object_added_id); + nm_clear_g_signal_handler (priv->modm.manager, &priv->modm.handle_object_removed_id); + g_clear_object (&priv->modm.manager); } static void -modem_object_added (MMManager *modem_manager, - MMObject *modem_object, - NMModemManager *self) +modm_handle_object_added (MMManager *modem_manager, + MMObject *modem_object, + NMModemManager *self) { NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); const gchar *path; @@ -171,9 +183,9 @@ modem_object_added (MMManager *modem_manager, } static void -modem_object_removed (MMManager *manager, - MMObject *modem_object, - NMModemManager *self) +modm_handle_object_removed (MMManager *manager, + MMObject *modem_object, + NMModemManager *self) { NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); NMModem *modem; @@ -189,7 +201,7 @@ modem_object_removed (MMManager *manager, } static void -modem_manager_available (NMModemManager *self) +modm_manager_available (NMModemManager *self) { NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); GList *modems, *l; @@ -197,26 +209,22 @@ modem_manager_available (NMModemManager *self) _LOGI ("ModemManager available in the bus"); /* Update initial modems list */ - modems = g_dbus_object_manager_get_objects (G_DBUS_OBJECT_MANAGER (priv->modem_manager)); + modems = g_dbus_object_manager_get_objects (G_DBUS_OBJECT_MANAGER (priv->modm.manager)); for (l = modems; l; l = g_list_next (l)) - modem_object_added (priv->modem_manager, MM_OBJECT (l->data), self); + modm_handle_object_added (priv->modm.manager, MM_OBJECT (l->data), self); g_list_free_full (modems, (GDestroyNotify) g_object_unref); } -static void schedule_modem_manager_relaunch (NMModemManager *self, - guint n_seconds); -static void ensure_modem_manager (NMModemManager *self); - static void -modem_manager_name_owner_changed (MMManager *modem_manager, - GParamSpec *pspec, - NMModemManager *self) +modm_handle_name_owner_changed (MMManager *modem_manager, + GParamSpec *pspec, + NMModemManager *self) { NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); gchar *name_owner; /* Quit poking, if any */ - nm_clear_g_source (&priv->mm_launch_id); + nm_clear_g_source (&priv->modm.relaunch_id); name_owner = g_dbus_object_manager_client_get_name_owner (G_DBUS_OBJECT_MANAGER_CLIENT (modem_manager)); if (!name_owner) { @@ -224,7 +232,7 @@ modem_manager_name_owner_changed (MMManager *modem_manager, /* If not managed by systemd, schedule relaunch */ if (!sd_booted ()) - schedule_modem_manager_relaunch (self, 0); + modm_schedule_manager_relaunch (self, 0); return; } @@ -236,14 +244,16 @@ modem_manager_name_owner_changed (MMManager *modem_manager, * nor 'object-removed' if it was created while there was no ModemManager in * the bus. This hack avoids this issue until we get a GIO with the fix * included... */ - clear_modem_manager (self); - ensure_modem_manager (self); + modm_clear_manager (self); + modm_ensure_manager (self); /* Whenever GDBusObjectManagerClient is fixed, we can just do the following: - * modem_manager_available (self); + * modm_manager_available (self); */ } +/*****************************************************************************/ + #if WITH_OFONO static void @@ -320,7 +330,7 @@ ofono_enumerate_devices_done (GObject *proxy, self = NM_MODEM_MANAGER (user_data); priv = NM_MODEM_MANAGER_GET_PRIVATE (self); - g_clear_object (&priv->ofono_cancellable); + g_clear_object (&priv->ofono.cancellable); if (!results) { _LOGW ("failed to enumerate oFono devices: %s", @@ -341,19 +351,19 @@ ofono_check_name_owner (NMModemManager *self) NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); gs_free char *name_owner = NULL; - name_owner = g_dbus_proxy_get_name_owner (G_DBUS_PROXY (priv->ofono_proxy)); + name_owner = g_dbus_proxy_get_name_owner (G_DBUS_PROXY (priv->ofono.proxy)); if (name_owner) { _LOGI ("oFono is now available"); - nm_clear_g_cancellable (&priv->ofono_cancellable); - priv->ofono_cancellable = g_cancellable_new (); + nm_clear_g_cancellable (&priv->ofono.cancellable); + priv->ofono.cancellable = g_cancellable_new (); - g_dbus_proxy_call (priv->ofono_proxy, + g_dbus_proxy_call (priv->ofono.proxy, "GetModems", NULL, G_DBUS_CALL_FLAGS_NONE, -1, - priv->ofono_cancellable, + priv->ofono.cancellable, ofono_enumerate_devices_done, self); } else { @@ -399,21 +409,21 @@ ofono_proxy_new_cb (GObject *source_object, self = NM_MODEM_MANAGER (user_data); priv = NM_MODEM_MANAGER_GET_PRIVATE (self); - g_clear_object (&priv->ofono_cancellable); + g_clear_object (&priv->ofono.cancellable); if (!proxy) { _LOGW ("error getting oFono bus proxy: %s", error->message); return; } - priv->ofono_proxy = proxy; + priv->ofono.proxy = proxy; - g_signal_connect (priv->ofono_proxy, + g_signal_connect (priv->ofono.proxy, "notify::g-name-owner", G_CALLBACK (ofono_name_owner_changed), self); - g_signal_connect (priv->ofono_proxy, + g_signal_connect (priv->ofono.proxy, "g-signal", G_CALLBACK (ofono_signal_cb), self); @@ -422,14 +432,14 @@ ofono_proxy_new_cb (GObject *source_object, } static void -ensure_ofono_client (NMModemManager *self) +ofono_init_proxy (NMModemManager *self) { NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); nm_assert (priv->dbus_connection); - nm_assert (!priv->ofono_cancellable); + nm_assert (!priv->ofono.cancellable); - priv->ofono_cancellable = g_cancellable_new (); + priv->ofono.cancellable = g_cancellable_new (); g_dbus_proxy_new (priv->dbus_connection, G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START, @@ -437,16 +447,18 @@ ensure_ofono_client (NMModemManager *self) OFONO_DBUS_SERVICE, OFONO_DBUS_PATH, OFONO_DBUS_INTERFACE, - priv->ofono_cancellable, + priv->ofono.cancellable, ofono_proxy_new_cb, self); } #endif +/*****************************************************************************/ + static void -modem_manager_poke_cb (GObject *connection, - GAsyncResult *res, - gpointer user_data) +modm_manager_poke_cb (GObject *connection, + GAsyncResult *res, + gpointer user_data) { NMModemManager *self; NMModemManagerPrivate *priv; @@ -462,7 +474,7 @@ modem_manager_poke_cb (GObject *connection, self = user_data; priv = NM_MODEM_MANAGER_GET_PRIVATE (self); - g_clear_object (&priv->poke_cancellable); + g_clear_object (&priv->modm.poke_cancellable); if (error) { _LOGW ("error poking ModemManager: %s", error->message); @@ -472,18 +484,18 @@ modem_manager_poke_cb (GObject *connection, && !g_error_matches (error, G_DBUS_ERROR, G_DBUS_ERROR_SPAWN_SERVICE_NOT_FOUND)) { /* Setup timeout to relaunch */ - schedule_modem_manager_relaunch (self, MODEM_POKE_INTERVAL); + modm_schedule_manager_relaunch (self, MODEM_POKE_INTERVAL); } } } static void -modem_manager_poke (NMModemManager *self) +modm_manager_poke (NMModemManager *self) { NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); - nm_clear_g_cancellable (&priv->poke_cancellable); - priv->poke_cancellable = g_cancellable_new (); + nm_clear_g_cancellable (&priv->modm.poke_cancellable); + priv->modm.poke_cancellable = g_cancellable_new (); /* If there is no current owner right away, ensure we poke to get one */ g_dbus_connection_call (priv->dbus_connection, @@ -491,37 +503,36 @@ modem_manager_poke (NMModemManager *self) "/org/freedesktop/ModemManager1", DBUS_INTERFACE_PEER, "Ping", - NULL, /* inputs */ - NULL, /* outputs */ + NULL, + NULL, G_DBUS_CALL_FLAGS_NONE, -1, - priv->poke_cancellable, - modem_manager_poke_cb, /* callback */ - self); /* user_data */ + priv->modm.poke_cancellable, + modm_manager_poke_cb, + self); } static void -modem_manager_check_name_owner (NMModemManager *self) +modm_manager_check_name_owner (NMModemManager *self) { NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); gs_free gchar *name_owner = NULL; - name_owner = g_dbus_object_manager_client_get_name_owner (G_DBUS_OBJECT_MANAGER_CLIENT (priv->modem_manager)); + name_owner = g_dbus_object_manager_client_get_name_owner (G_DBUS_OBJECT_MANAGER_CLIENT (priv->modm.manager)); if (name_owner) { - /* Available! */ - modem_manager_available (self); + modm_manager_available (self); return; } /* If the lifecycle is not managed by systemd, poke */ if (!sd_booted ()) - modem_manager_poke (self); + modm_manager_poke (self); } static void -manager_new_ready (GObject *source, - GAsyncResult *res, - gpointer user_data) +modm_manager_new_cb (GObject *source, + GAsyncResult *res, + gpointer user_data) { NMModemManager *self; NMModemManagerPrivate *priv; @@ -536,7 +547,7 @@ manager_new_ready (GObject *source, self = user_data; priv = NM_MODEM_MANAGER_GET_PRIVATE (self); - nm_assert (!priv->modem_manager); + nm_assert (!priv->modm.manager); g_clear_object (&priv->main_cancellable); @@ -546,34 +557,34 @@ manager_new_ready (GObject *source, * During this period, name-owner changes won't be followed. */ _LOGW ("error creating ModemManager client: %s", error->message); /* Setup timeout to relaunch */ - schedule_modem_manager_relaunch (self, MODEM_POKE_INTERVAL); + modm_schedule_manager_relaunch (self, MODEM_POKE_INTERVAL); return; } - priv->modem_manager = modem_manager; + priv->modm.manager = modem_manager; /* Setup signals in the GDBusObjectManagerClient */ - priv->mm_name_owner_changed_id = - g_signal_connect (priv->modem_manager, + priv->modm.handle_name_owner_changed_id = + g_signal_connect (priv->modm.manager, "notify::name-owner", - G_CALLBACK (modem_manager_name_owner_changed), + G_CALLBACK (modm_handle_name_owner_changed), self); - priv->mm_object_added_id = - g_signal_connect (priv->modem_manager, + priv->modm.handle_object_added_id = + g_signal_connect (priv->modm.manager, "object-added", - G_CALLBACK (modem_object_added), + G_CALLBACK (modm_handle_object_added), self); - priv->mm_object_removed_id = - g_signal_connect (priv->modem_manager, + priv->modm.handle_object_removed_id = + g_signal_connect (priv->modm.manager, "object-removed", - G_CALLBACK (modem_object_removed), + G_CALLBACK (modm_handle_object_removed), self); - modem_manager_check_name_owner (self); + modm_manager_check_name_owner (self); } static void -ensure_modem_manager (NMModemManager *self) +modm_ensure_manager (NMModemManager *self) { NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); @@ -582,45 +593,47 @@ ensure_modem_manager (NMModemManager *self) /* Create the GDBusObjectManagerClient. We do not request to autostart, as * we don't really want the MMManager creation to fail. We can always poke * later on if we want to request the autostart */ - if (!priv->modem_manager) { + if (!priv->modm.manager) { if (!priv->main_cancellable) priv->main_cancellable = g_cancellable_new (); mm_manager_new (priv->dbus_connection, G_DBUS_OBJECT_MANAGER_CLIENT_FLAGS_DO_NOT_AUTO_START, priv->main_cancellable, - manager_new_ready, + modm_manager_new_cb, self); return; } /* If already available, recheck name owner! */ - modem_manager_check_name_owner (self); + modm_manager_check_name_owner (self); } static gboolean -mm_launch_cb (NMModemManager *self) +modm_schedule_manager_relaunch_cb (NMModemManager *self) { NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); - priv->mm_launch_id = 0; - ensure_modem_manager (self); + priv->modm.relaunch_id = 0; + modm_ensure_manager (self); return G_SOURCE_REMOVE; } static void -schedule_modem_manager_relaunch (NMModemManager *self, - guint n_seconds) +modm_schedule_manager_relaunch (NMModemManager *self, + guint n_seconds) { NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); /* No need to pass an extra reference to self; timeout/idle will be * cancelled if the object gets disposed. */ if (n_seconds) - priv->mm_launch_id = g_timeout_add_seconds (n_seconds, (GSourceFunc)mm_launch_cb, self); + priv->modm.relaunch_id = g_timeout_add_seconds (n_seconds, (GSourceFunc)modm_schedule_manager_relaunch_cb, self); else - priv->mm_launch_id = g_idle_add ((GSourceFunc)mm_launch_cb, self); + priv->modm.relaunch_id = g_idle_add ((GSourceFunc)modm_schedule_manager_relaunch_cb, self); } +/*****************************************************************************/ + static void bus_get_ready (GObject *source, GAsyncResult *res, @@ -646,10 +659,9 @@ bus_get_ready (GObject *source, priv->dbus_connection = connection; - /* Got the bus, ensure clients */ - ensure_modem_manager (self); + modm_ensure_manager (self); #if WITH_OFONO - ensure_ofono_client (self); + ofono_init_proxy (self); #endif } @@ -677,19 +689,19 @@ dispose (GObject *object) NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); nm_clear_g_cancellable (&priv->main_cancellable); - nm_clear_g_cancellable (&priv->poke_cancellable); + nm_clear_g_cancellable (&priv->modm.poke_cancellable); - nm_clear_g_source (&priv->mm_launch_id); + nm_clear_g_source (&priv->modm.relaunch_id); - clear_modem_manager (self); + modm_clear_manager (self); #if WITH_OFONO - if (priv->ofono_proxy) { - g_signal_handlers_disconnect_by_func (priv->ofono_proxy, ofono_name_owner_changed, self); - g_signal_handlers_disconnect_by_func (priv->ofono_proxy, ofono_signal_cb, self); - g_clear_object (&priv->ofono_proxy); + if (priv->ofono.proxy) { + g_signal_handlers_disconnect_by_func (priv->ofono.proxy, ofono_name_owner_changed, self); + g_signal_handlers_disconnect_by_func (priv->ofono.proxy, ofono_signal_cb, self); + g_clear_object (&priv->ofono.proxy); } - nm_clear_g_cancellable (&priv->ofono_cancellable); + nm_clear_g_cancellable (&priv->ofono.cancellable); #endif g_clear_object (&priv->dbus_connection); -- cgit v1.2.1 From 2dbb4d8d2b551c93a643d9b9c222e22d706cb932 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 22 Apr 2017 09:56:26 +0200 Subject: modem/trivial: move code around --- src/devices/wwan/nm-modem-manager.c | 356 ++++++++++++++++++------------------ 1 file changed, 177 insertions(+), 179 deletions(-) diff --git a/src/devices/wwan/nm-modem-manager.c b/src/devices/wwan/nm-modem-manager.c index 94a39d6493..613bd84ede 100644 --- a/src/devices/wwan/nm-modem-manager.c +++ b/src/devices/wwan/nm-modem-manager.c @@ -252,6 +252,183 @@ modm_handle_name_owner_changed (MMManager *modem_manager, */ } +static void +modm_manager_poke_cb (GObject *connection, + GAsyncResult *res, + gpointer user_data) +{ + NMModemManager *self; + NMModemManagerPrivate *priv; + gs_free_error GError *error = NULL; + gs_unref_variant GVariant *result = NULL; + + result = g_dbus_connection_call_finish (G_DBUS_CONNECTION (connection), res, &error); + + if ( !result + && g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + return; + + self = user_data; + priv = NM_MODEM_MANAGER_GET_PRIVATE (self); + + g_clear_object (&priv->modm.poke_cancellable); + + if (error) { + _LOGW ("error poking ModemManager: %s", error->message); + + /* Don't reschedule poke is MM service doesn't exist. */ + if ( !g_error_matches (error, G_DBUS_ERROR, G_DBUS_ERROR_SERVICE_UNKNOWN) + && !g_error_matches (error, G_DBUS_ERROR, G_DBUS_ERROR_SPAWN_SERVICE_NOT_FOUND)) { + + /* Setup timeout to relaunch */ + modm_schedule_manager_relaunch (self, MODEM_POKE_INTERVAL); + } + } +} + +static void +modm_manager_poke (NMModemManager *self) +{ + NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); + + nm_clear_g_cancellable (&priv->modm.poke_cancellable); + priv->modm.poke_cancellable = g_cancellable_new (); + + /* If there is no current owner right away, ensure we poke to get one */ + g_dbus_connection_call (priv->dbus_connection, + "org.freedesktop.ModemManager1", + "/org/freedesktop/ModemManager1", + DBUS_INTERFACE_PEER, + "Ping", + NULL, + NULL, + G_DBUS_CALL_FLAGS_NONE, + -1, + priv->modm.poke_cancellable, + modm_manager_poke_cb, + self); +} + +static void +modm_manager_check_name_owner (NMModemManager *self) +{ + NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); + gs_free gchar *name_owner = NULL; + + name_owner = g_dbus_object_manager_client_get_name_owner (G_DBUS_OBJECT_MANAGER_CLIENT (priv->modm.manager)); + if (name_owner) { + modm_manager_available (self); + return; + } + + /* If the lifecycle is not managed by systemd, poke */ + if (!sd_booted ()) + modm_manager_poke (self); +} + +static void +modm_manager_new_cb (GObject *source, + GAsyncResult *res, + gpointer user_data) +{ + NMModemManager *self; + NMModemManagerPrivate *priv; + gs_free_error GError *error = NULL; + MMManager *modem_manager; + + modem_manager = mm_manager_new_finish (res, &error); + if ( !modem_manager + && g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + return; + + self = user_data; + priv = NM_MODEM_MANAGER_GET_PRIVATE (self); + + nm_assert (!priv->modm.manager); + + g_clear_object (&priv->main_cancellable); + + if (!modem_manager) { + /* We're not really supposed to get any error here. If we do get one, + * though, just re-schedule the MMManager creation after some time. + * During this period, name-owner changes won't be followed. */ + _LOGW ("error creating ModemManager client: %s", error->message); + /* Setup timeout to relaunch */ + modm_schedule_manager_relaunch (self, MODEM_POKE_INTERVAL); + return; + } + + priv->modm.manager = modem_manager; + + /* Setup signals in the GDBusObjectManagerClient */ + priv->modm.handle_name_owner_changed_id = + g_signal_connect (priv->modm.manager, + "notify::name-owner", + G_CALLBACK (modm_handle_name_owner_changed), + self); + priv->modm.handle_object_added_id = + g_signal_connect (priv->modm.manager, + "object-added", + G_CALLBACK (modm_handle_object_added), + self); + priv->modm.handle_object_removed_id = + g_signal_connect (priv->modm.manager, + "object-removed", + G_CALLBACK (modm_handle_object_removed), + self); + + modm_manager_check_name_owner (self); +} + +static void +modm_ensure_manager (NMModemManager *self) +{ + NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); + + g_assert (priv->dbus_connection); + + /* Create the GDBusObjectManagerClient. We do not request to autostart, as + * we don't really want the MMManager creation to fail. We can always poke + * later on if we want to request the autostart */ + if (!priv->modm.manager) { + if (!priv->main_cancellable) + priv->main_cancellable = g_cancellable_new (); + mm_manager_new (priv->dbus_connection, + G_DBUS_OBJECT_MANAGER_CLIENT_FLAGS_DO_NOT_AUTO_START, + priv->main_cancellable, + modm_manager_new_cb, + self); + return; + } + + /* If already available, recheck name owner! */ + modm_manager_check_name_owner (self); +} + +static gboolean +modm_schedule_manager_relaunch_cb (NMModemManager *self) +{ + NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); + + priv->modm.relaunch_id = 0; + modm_ensure_manager (self); + return G_SOURCE_REMOVE; +} + +static void +modm_schedule_manager_relaunch (NMModemManager *self, + guint n_seconds) +{ + NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); + + /* No need to pass an extra reference to self; timeout/idle will be + * cancelled if the object gets disposed. */ + if (n_seconds) + priv->modm.relaunch_id = g_timeout_add_seconds (n_seconds, (GSourceFunc)modm_schedule_manager_relaunch_cb, self); + else + priv->modm.relaunch_id = g_idle_add ((GSourceFunc)modm_schedule_manager_relaunch_cb, self); +} + /*****************************************************************************/ #if WITH_OFONO @@ -455,185 +632,6 @@ ofono_init_proxy (NMModemManager *self) /*****************************************************************************/ -static void -modm_manager_poke_cb (GObject *connection, - GAsyncResult *res, - gpointer user_data) -{ - NMModemManager *self; - NMModemManagerPrivate *priv; - gs_free_error GError *error = NULL; - gs_unref_variant GVariant *result = NULL; - - result = g_dbus_connection_call_finish (G_DBUS_CONNECTION (connection), res, &error); - - if ( !result - && g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) - return; - - self = user_data; - priv = NM_MODEM_MANAGER_GET_PRIVATE (self); - - g_clear_object (&priv->modm.poke_cancellable); - - if (error) { - _LOGW ("error poking ModemManager: %s", error->message); - - /* Don't reschedule poke is MM service doesn't exist. */ - if ( !g_error_matches (error, G_DBUS_ERROR, G_DBUS_ERROR_SERVICE_UNKNOWN) - && !g_error_matches (error, G_DBUS_ERROR, G_DBUS_ERROR_SPAWN_SERVICE_NOT_FOUND)) { - - /* Setup timeout to relaunch */ - modm_schedule_manager_relaunch (self, MODEM_POKE_INTERVAL); - } - } -} - -static void -modm_manager_poke (NMModemManager *self) -{ - NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); - - nm_clear_g_cancellable (&priv->modm.poke_cancellable); - priv->modm.poke_cancellable = g_cancellable_new (); - - /* If there is no current owner right away, ensure we poke to get one */ - g_dbus_connection_call (priv->dbus_connection, - "org.freedesktop.ModemManager1", - "/org/freedesktop/ModemManager1", - DBUS_INTERFACE_PEER, - "Ping", - NULL, - NULL, - G_DBUS_CALL_FLAGS_NONE, - -1, - priv->modm.poke_cancellable, - modm_manager_poke_cb, - self); -} - -static void -modm_manager_check_name_owner (NMModemManager *self) -{ - NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); - gs_free gchar *name_owner = NULL; - - name_owner = g_dbus_object_manager_client_get_name_owner (G_DBUS_OBJECT_MANAGER_CLIENT (priv->modm.manager)); - if (name_owner) { - modm_manager_available (self); - return; - } - - /* If the lifecycle is not managed by systemd, poke */ - if (!sd_booted ()) - modm_manager_poke (self); -} - -static void -modm_manager_new_cb (GObject *source, - GAsyncResult *res, - gpointer user_data) -{ - NMModemManager *self; - NMModemManagerPrivate *priv; - gs_free_error GError *error = NULL; - MMManager *modem_manager; - - modem_manager = mm_manager_new_finish (res, &error); - if ( !modem_manager - && g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) - return; - - self = user_data; - priv = NM_MODEM_MANAGER_GET_PRIVATE (self); - - nm_assert (!priv->modm.manager); - - g_clear_object (&priv->main_cancellable); - - if (!modem_manager) { - /* We're not really supposed to get any error here. If we do get one, - * though, just re-schedule the MMManager creation after some time. - * During this period, name-owner changes won't be followed. */ - _LOGW ("error creating ModemManager client: %s", error->message); - /* Setup timeout to relaunch */ - modm_schedule_manager_relaunch (self, MODEM_POKE_INTERVAL); - return; - } - - priv->modm.manager = modem_manager; - - /* Setup signals in the GDBusObjectManagerClient */ - priv->modm.handle_name_owner_changed_id = - g_signal_connect (priv->modm.manager, - "notify::name-owner", - G_CALLBACK (modm_handle_name_owner_changed), - self); - priv->modm.handle_object_added_id = - g_signal_connect (priv->modm.manager, - "object-added", - G_CALLBACK (modm_handle_object_added), - self); - priv->modm.handle_object_removed_id = - g_signal_connect (priv->modm.manager, - "object-removed", - G_CALLBACK (modm_handle_object_removed), - self); - - modm_manager_check_name_owner (self); -} - -static void -modm_ensure_manager (NMModemManager *self) -{ - NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); - - g_assert (priv->dbus_connection); - - /* Create the GDBusObjectManagerClient. We do not request to autostart, as - * we don't really want the MMManager creation to fail. We can always poke - * later on if we want to request the autostart */ - if (!priv->modm.manager) { - if (!priv->main_cancellable) - priv->main_cancellable = g_cancellable_new (); - mm_manager_new (priv->dbus_connection, - G_DBUS_OBJECT_MANAGER_CLIENT_FLAGS_DO_NOT_AUTO_START, - priv->main_cancellable, - modm_manager_new_cb, - self); - return; - } - - /* If already available, recheck name owner! */ - modm_manager_check_name_owner (self); -} - -static gboolean -modm_schedule_manager_relaunch_cb (NMModemManager *self) -{ - NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); - - priv->modm.relaunch_id = 0; - modm_ensure_manager (self); - return G_SOURCE_REMOVE; -} - -static void -modm_schedule_manager_relaunch (NMModemManager *self, - guint n_seconds) -{ - NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); - - /* No need to pass an extra reference to self; timeout/idle will be - * cancelled if the object gets disposed. */ - if (n_seconds) - priv->modm.relaunch_id = g_timeout_add_seconds (n_seconds, (GSourceFunc)modm_schedule_manager_relaunch_cb, self); - else - priv->modm.relaunch_id = g_idle_add ((GSourceFunc)modm_schedule_manager_relaunch_cb, self); -} - -/*****************************************************************************/ - static void bus_get_ready (GObject *source, GAsyncResult *res, -- cgit v1.2.1 From c257e22cb59e427b465841992154de1712e1f4eb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 22 Apr 2017 10:27:23 +0200 Subject: modem: cleanup construction of NMModem It is invalid that a constructor() returns NULL. These anyway were only assertions, checking conditions that should never fail. --- src/devices/wwan/nm-modem-ofono.c | 4 ++-- src/devices/wwan/nm-modem.c | 39 ++++++++------------------------------- 2 files changed, 10 insertions(+), 33 deletions(-) diff --git a/src/devices/wwan/nm-modem-ofono.c b/src/devices/wwan/nm-modem-ofono.c index 52b335c7bc..714f9e8348 100644 --- a/src/devices/wwan/nm-modem-ofono.c +++ b/src/devices/wwan/nm-modem-ofono.c @@ -197,8 +197,8 @@ disconnect_finish (NMModem *self, static void disconnect_done (GDBusProxy *proxy, - GAsyncResult *result, - gpointer user_data) + GAsyncResult *result, + gpointer user_data) { DisconnectContext *ctx = (DisconnectContext*) user_data; NMModemOfono *self = ctx->self; diff --git a/src/devices/wwan/nm-modem.c b/src/devices/wwan/nm-modem.c index 6494b84995..c6885cef7d 100644 --- a/src/devices/wwan/nm-modem.c +++ b/src/devices/wwan/nm-modem.c @@ -1451,6 +1451,7 @@ set_property (GObject *object, guint prop_id, case PROP_PATH: /* construct-only */ priv->path = g_value_dup_string (value); + g_return_if_fail (priv->path); break; case PROP_DRIVER: /* construct-only */ @@ -1512,37 +1513,16 @@ nm_modem_init (NMModem *self) self->_priv = G_TYPE_INSTANCE_GET_PRIVATE (self, NM_TYPE_MODEM, NMModemPrivate); } -static GObject* -constructor (GType type, - guint n_construct_params, - GObjectConstructParam *construct_params) +static void +constructed (GObject *object) { - GObject *object; NMModemPrivate *priv; - object = G_OBJECT_CLASS (nm_modem_parent_class)->constructor (type, - n_construct_params, - construct_params); - if (!object) - return NULL; - - priv = NM_MODEM_GET_PRIVATE ((NMModem *) object); - - if (!priv->data_port && !priv->control_port) { - nm_log_err (LOGD_PLATFORM, "neither modem command nor data interface provided"); - goto err; - } - - if (!priv->path) { - nm_log_err (LOGD_PLATFORM, "D-Bus path not provided"); - goto err; - } + G_OBJECT_CLASS (nm_modem_parent_class)->constructed (object); - return object; + priv = NM_MODEM_GET_PRIVATE (NM_MODEM (object)); -err: - g_object_unref (object); - return NULL; + g_return_if_fail (priv->data_port || priv->control_port); } /*****************************************************************************/ @@ -1552,10 +1532,7 @@ dispose (GObject *object) { NMModemPrivate *priv = NM_MODEM_GET_PRIVATE ((NMModem *) object); - if (priv->act_request) { - g_object_unref (priv->act_request); - priv->act_request = NULL; - } + g_clear_object (&priv->act_request); G_OBJECT_CLASS (nm_modem_parent_class)->dispose (object); } @@ -1584,7 +1561,7 @@ nm_modem_class_init (NMModemClass *klass) g_type_class_add_private (object_class, sizeof (NMModemPrivate)); - object_class->constructor = constructor; + object_class->constructed = constructed; object_class->set_property = set_property; object_class->get_property = get_property; object_class->dispose = dispose; -- cgit v1.2.1 From ce1ae5f4584c3bef1f94fdb4b983c38f53e65d9e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 22 Apr 2017 10:53:08 +0200 Subject: modem: add define for ModemManager D-Bus path Also, bluetooth plugin uses NMModem from the wwan plugin. Don't include such a foreign header in a "nm-device-bt.h". Instead, forward declare what we need. --- src/devices/bluetooth/nm-device-bt.c | 15 +++++++-------- src/devices/bluetooth/nm-device-bt.h | 6 +++--- src/devices/wwan/nm-modem-manager.c | 4 ++-- src/devices/wwan/nm-modem-manager.h | 4 ++++ src/devices/wwan/nm-modem.h | 6 ++++-- 5 files changed, 20 insertions(+), 15 deletions(-) diff --git a/src/devices/bluetooth/nm-device-bt.c b/src/devices/bluetooth/nm-device-bt.c index 4ee7148928..7f20238972 100644 --- a/src/devices/bluetooth/nm-device-bt.c +++ b/src/devices/bluetooth/nm-device-bt.c @@ -40,15 +40,14 @@ #include "nm-bt-error.h" #include "platform/nm-platform.h" +#include "devices/wwan/nm-modem-manager.h" +#include "devices/wwan/nm-modem.h" + #include "introspection/org.freedesktop.NetworkManager.Device.Bluetooth.h" #include "devices/nm-device-logging.h" _LOG_DECLARE_SELF(NMDeviceBt); -#define MM_DBUS_SERVICE "org.freedesktop.ModemManager1" -#define MM_DBUS_PATH "/org/freedesktop/ModemManager1" -#define MM_DBUS_INTERFACE "org.freedesktop.ModemManager1" - /*****************************************************************************/ NM_GOBJECT_PROPERTIES_DEFINE_BASE ( @@ -1060,9 +1059,9 @@ nm_device_bt_init (NMDeviceBt *self) G_DBUS_PROXY_FLAGS_DO_NOT_CONNECT_SIGNALS | G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START, NULL, - MM_DBUS_SERVICE, - MM_DBUS_PATH, - MM_DBUS_INTERFACE, + NM_MODEM_MANAGER_MM_DBUS_SERVICE, + NM_MODEM_MANAGER_MM_DBUS_PATH, + NM_MODEM_MANAGER_MM_DBUS_INTERFACE, NULL, &error); if (priv->mm_proxy) { g_signal_connect (priv->mm_proxy, "notify::g-name-owner", @@ -1071,7 +1070,7 @@ nm_device_bt_init (NMDeviceBt *self) mm_name_owner_changed (G_OBJECT (priv->mm_proxy), NULL, self); } else { _LOGW (LOGD_MB, "Could not create proxy for '%s': %s", - MM_DBUS_SERVICE, error->message); + NM_MODEM_MANAGER_MM_DBUS_SERVICE, error->message); g_clear_error (&error); } } diff --git a/src/devices/bluetooth/nm-device-bt.h b/src/devices/bluetooth/nm-device-bt.h index 9bcf6ca861..b90dbd2aa9 100644 --- a/src/devices/bluetooth/nm-device-bt.h +++ b/src/devices/bluetooth/nm-device-bt.h @@ -24,8 +24,6 @@ #include "devices/nm-device.h" #include "nm-bluez-device.h" -#include "devices/wwan/nm-modem.h" - #define NM_TYPE_DEVICE_BT (nm_device_bt_get_type ()) #define NM_DEVICE_BT(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), NM_TYPE_DEVICE_BT, NMDeviceBt)) #define NM_DEVICE_BT_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), NM_TYPE_DEVICE_BT, NMDeviceBtClass)) @@ -52,8 +50,10 @@ NMDevice *nm_device_bt_new (NMBluezDevice *bt_device, guint32 nm_device_bt_get_capabilities (NMDeviceBt *device); +struct _NMModem; + gboolean nm_device_bt_modem_added (NMDeviceBt *device, - NMModem *modem, + struct _NMModem *modem, const char *driver); #endif /* __NETWORKMANAGER_DEVICE_BT_H__ */ diff --git a/src/devices/wwan/nm-modem-manager.c b/src/devices/wwan/nm-modem-manager.c index 613bd84ede..381a909c18 100644 --- a/src/devices/wwan/nm-modem-manager.c +++ b/src/devices/wwan/nm-modem-manager.c @@ -296,8 +296,8 @@ modm_manager_poke (NMModemManager *self) /* If there is no current owner right away, ensure we poke to get one */ g_dbus_connection_call (priv->dbus_connection, - "org.freedesktop.ModemManager1", - "/org/freedesktop/ModemManager1", + NM_MODEM_MANAGER_MM_DBUS_SERVICE, + NM_MODEM_MANAGER_MM_DBUS_PATH, DBUS_INTERFACE_PEER, "Ping", NULL, diff --git a/src/devices/wwan/nm-modem-manager.h b/src/devices/wwan/nm-modem-manager.h index 65594dfa56..70feee9109 100644 --- a/src/devices/wwan/nm-modem-manager.h +++ b/src/devices/wwan/nm-modem-manager.h @@ -34,6 +34,10 @@ #define NM_MODEM_MANAGER_MODEM_ADDED "modem-added" +#define NM_MODEM_MANAGER_MM_DBUS_SERVICE "org.freedesktop.ModemManager1" +#define NM_MODEM_MANAGER_MM_DBUS_PATH "/org/freedesktop/ModemManager1" +#define NM_MODEM_MANAGER_MM_DBUS_INTERFACE "org.freedesktop.ModemManager1" + typedef struct _NMModemManager NMModemManager; typedef struct _NMModemManagerClass NMModemManagerClass; diff --git a/src/devices/wwan/nm-modem.h b/src/devices/wwan/nm-modem.h index a50727a96a..c6efda951d 100644 --- a/src/devices/wwan/nm-modem.h +++ b/src/devices/wwan/nm-modem.h @@ -105,10 +105,12 @@ typedef enum { /*< underscore_name=nm_modem_state >*/ struct _NMModemPrivate; -typedef struct { +struct _NMModem { GObject parent; struct _NMModemPrivate *_priv; -} NMModem; +}; + +typedef struct _NMModem NMModem; typedef struct { GObjectClass parent; -- cgit v1.2.1 From a087278e8ebf2e2f8b9e96aa9814468e1cf4e469 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 22 Apr 2017 10:58:14 +0200 Subject: modem: make NMModemManager a singleton Singletons are not entirely bad, if used carefully. We will need the singleton from bluetooth plugin. --- src/devices/wwan/nm-modem-manager.c | 4 ++++ src/devices/wwan/nm-modem-manager.h | 2 ++ src/devices/wwan/nm-wwan-factory.c | 4 ++-- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/devices/wwan/nm-modem-manager.c b/src/devices/wwan/nm-modem-manager.c index 381a909c18..d6a515077a 100644 --- a/src/devices/wwan/nm-modem-manager.c +++ b/src/devices/wwan/nm-modem-manager.c @@ -97,6 +97,10 @@ G_DEFINE_TYPE (NMModemManager, nm_modem_manager, G_TYPE_OBJECT) /*****************************************************************************/ +NM_DEFINE_SINGLETON_GETTER (NMModemManager, nm_modem_manager_get, NM_TYPE_MODEM_MANAGER); + +/*****************************************************************************/ + static void modm_schedule_manager_relaunch (NMModemManager *self, guint n_seconds); static void modm_ensure_manager (NMModemManager *self); diff --git a/src/devices/wwan/nm-modem-manager.h b/src/devices/wwan/nm-modem-manager.h index 70feee9109..84eb232e98 100644 --- a/src/devices/wwan/nm-modem-manager.h +++ b/src/devices/wwan/nm-modem-manager.h @@ -43,4 +43,6 @@ typedef struct _NMModemManagerClass NMModemManagerClass; GType nm_modem_manager_get_type (void); +NMModemManager *nm_modem_manager_get (void); + #endif /* __NETWORKMANAGER_MODEM_MANAGER_H__ */ diff --git a/src/devices/wwan/nm-wwan-factory.c b/src/devices/wwan/nm-wwan-factory.c index fa4c8dbbf6..663102de4d 100644 --- a/src/devices/wwan/nm-wwan-factory.c +++ b/src/devices/wwan/nm-wwan-factory.c @@ -127,8 +127,8 @@ start (NMDeviceFactory *factory) NMWwanFactory *self = NM_WWAN_FACTORY (factory); NMWwanFactoryPrivate *priv = NM_WWAN_FACTORY_GET_PRIVATE (self); - priv->mm = g_object_new (NM_TYPE_MODEM_MANAGER, NULL); - g_assert (priv->mm); + priv->mm = g_object_ref (nm_modem_manager_get ()); + g_signal_connect (priv->mm, NM_MODEM_MANAGER_MODEM_ADDED, G_CALLBACK (modem_added_cb), -- cgit v1.2.1 From d1ef7f7aaca35b223c203ce6184aed8a13b67353 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 21 Apr 2017 19:59:40 +0200 Subject: bt: move initialization of NMDeviceBt to constructed() Don't do non-trivial initialization in nm_device_bt_init(). At this point, the object is not yet fully created. As we already have a constructed() implemented, move the initialization there. --- src/devices/bluetooth/nm-device-bt.c | 38 +++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/src/devices/bluetooth/nm-device-bt.c b/src/devices/bluetooth/nm-device-bt.c index 7f20238972..04d4927626 100644 --- a/src/devices/bluetooth/nm-device-bt.c +++ b/src/devices/bluetooth/nm-device-bt.c @@ -1038,7 +1038,8 @@ set_property (GObject *object, guint prop_id, case PROP_BT_DEVICE: /* construct-only */ priv->bt_device = g_value_dup_object (value); - g_signal_connect (priv->bt_device, "removed", G_CALLBACK (bluez_device_removed), object); + if (!priv->bt_device) + g_return_if_reached (); break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); @@ -1051,8 +1052,15 @@ set_property (GObject *object, guint prop_id, static void nm_device_bt_init (NMDeviceBt *self) { +} + +static void +constructed (GObject *object) +{ + NMDeviceBt *self = NM_DEVICE_BT (object); NMDeviceBtPrivate *priv = NM_DEVICE_BT_GET_PRIVATE (self); - GError *error = NULL; + const char *my_hwaddr; + gs_free_error GError *error = NULL; priv->mm_proxy = g_dbus_proxy_new_for_bus_sync (G_BUS_TYPE_SYSTEM, G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES | @@ -1073,24 +1081,22 @@ nm_device_bt_init (NMDeviceBt *self) NM_MODEM_MANAGER_MM_DBUS_SERVICE, error->message); g_clear_error (&error); } -} - -static void -constructed (GObject *object) -{ - NMDeviceBtPrivate *priv = NM_DEVICE_BT_GET_PRIVATE ((NMDeviceBt *) object); - const char *my_hwaddr; G_OBJECT_CLASS (nm_device_bt_parent_class)->constructed (object); - my_hwaddr = nm_device_get_hw_address (NM_DEVICE (object)); - g_assert (my_hwaddr); - priv->bdaddr = g_strdup (my_hwaddr); + if (priv->bt_device) { + /* Watch for BT device property changes */ + g_signal_connect (priv->bt_device, "notify::" NM_BLUEZ_DEVICE_CONNECTED, + G_CALLBACK (bluez_connected_changed), + object); + g_signal_connect (priv->bt_device, "removed", G_CALLBACK (bluez_device_removed), object); + } - /* Watch for BT device property changes */ - g_signal_connect (priv->bt_device, "notify::" NM_BLUEZ_DEVICE_CONNECTED, - G_CALLBACK (bluez_connected_changed), - object); + my_hwaddr = nm_device_get_hw_address (NM_DEVICE (object)); + if (my_hwaddr) + priv->bdaddr = g_strdup (my_hwaddr); + else + g_warn_if_reached (); } NMDevice * -- cgit v1.2.1 From 7840dde47b14ba0dc5b824689cd4d5f5e00c8591 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 21 Apr 2017 20:20:40 +0200 Subject: bt: minor refactoring of mm_name_owner_changed() function --- src/devices/bluetooth/nm-device-bt.c | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/src/devices/bluetooth/nm-device-bt.c b/src/devices/bluetooth/nm-device-bt.c index 04d4927626..86ee7f9780 100644 --- a/src/devices/bluetooth/nm-device-bt.c +++ b/src/devices/bluetooth/nm-device-bt.c @@ -982,18 +982,21 @@ set_mm_running (NMDeviceBt *self, gboolean running) } static void -mm_name_owner_changed (GObject *object, - GParamSpec *pspec, - NMDeviceBt *self) +mm_name_owner_changed (NMDeviceBt *self) { - char *owner; + NMDeviceBtPrivate *priv = NM_DEVICE_BT_GET_PRIVATE (self); + gs_free char *owner = NULL; - owner = g_dbus_proxy_get_name_owner (G_DBUS_PROXY (object)); - if (owner) { - set_mm_running (self, TRUE); - g_free (owner); - } else - set_mm_running (self, FALSE); + owner = g_dbus_proxy_get_name_owner (priv->mm_proxy); + set_mm_running (self, owner != NULL); +} + +static void +mm_name_owner_changed_cb (GObject *object, + GParamSpec *pspec, + NMDeviceBt *self) +{ + mm_name_owner_changed (self); } /*****************************************************************************/ @@ -1073,9 +1076,9 @@ constructed (GObject *object) NULL, &error); if (priv->mm_proxy) { g_signal_connect (priv->mm_proxy, "notify::g-name-owner", - G_CALLBACK (mm_name_owner_changed), + G_CALLBACK (mm_name_owner_changed_cb), self); - mm_name_owner_changed (G_OBJECT (priv->mm_proxy), NULL, self); + mm_name_owner_changed (self); } else { _LOGW (LOGD_MB, "Could not create proxy for '%s': %s", NM_MODEM_MANAGER_MM_DBUS_SERVICE, error->message); @@ -1135,7 +1138,7 @@ dispose (GObject *object) g_signal_handlers_disconnect_matched (priv->bt_device, G_SIGNAL_MATCH_DATA, 0, 0, NULL, NULL, object); if (priv->mm_proxy) { - g_signal_handlers_disconnect_by_func (priv->mm_proxy, G_CALLBACK (mm_name_owner_changed), object); + g_signal_handlers_disconnect_by_func (priv->mm_proxy, G_CALLBACK (mm_name_owner_changed_cb), object); g_clear_object (&priv->mm_proxy); } -- cgit v1.2.1 From e84a52ea42a959cfa4e52df6c346772847d1c329 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 22 Apr 2017 11:05:31 +0200 Subject: bt: track name-owner changes via NMModemManager and create D-Bus proxy asynchronously Fix two issues of the previous code: - the D-Bus proxy for the modem manager should not get created synchronously. - NMModemManager is a singleton, let it track the name-owner change and the D-Bus proxy, instead of having one per NMDeviceBt. --- src/devices/bluetooth/nm-device-bt.c | 61 +++++-------- src/devices/wwan/libnm-wwan.ver | 4 + src/devices/wwan/nm-modem-manager.c | 164 +++++++++++++++++++++++++++++++++++ src/devices/wwan/nm-modem-manager.h | 7 ++ 4 files changed, 198 insertions(+), 38 deletions(-) diff --git a/src/devices/bluetooth/nm-device-bt.c b/src/devices/bluetooth/nm-device-bt.c index 86ee7f9780..3402532fca 100644 --- a/src/devices/bluetooth/nm-device-bt.c +++ b/src/devices/bluetooth/nm-device-bt.c @@ -64,7 +64,8 @@ enum { static guint signals[LAST_SIGNAL] = { 0 }; typedef struct { - GDBusProxy *mm_proxy; + NMModemManager *modem_manager; + gboolean mm_running; NMBluezDevice *bt_device; @@ -966,9 +967,12 @@ is_available (NMDevice *dev, NMDeviceCheckDevAvailableFlags flags) } static void -set_mm_running (NMDeviceBt *self, gboolean running) +set_mm_running (NMDeviceBt *self) { NMDeviceBtPrivate *priv = NM_DEVICE_BT_GET_PRIVATE (self); + gboolean running; + + running = (nm_modem_manager_name_owner_get (priv->modem_manager) != NULL); if (priv->mm_running != running) { _LOGD (LOGD_BT, "ModemManager now %s", @@ -981,22 +985,12 @@ set_mm_running (NMDeviceBt *self, gboolean running) } } -static void -mm_name_owner_changed (NMDeviceBt *self) -{ - NMDeviceBtPrivate *priv = NM_DEVICE_BT_GET_PRIVATE (self); - gs_free char *owner = NULL; - - owner = g_dbus_proxy_get_name_owner (priv->mm_proxy); - set_mm_running (self, owner != NULL); -} - static void mm_name_owner_changed_cb (GObject *object, GParamSpec *pspec, - NMDeviceBt *self) + gpointer user_data) { - mm_name_owner_changed (self); + set_mm_running (user_data); } /*****************************************************************************/ @@ -1063,30 +1057,18 @@ constructed (GObject *object) NMDeviceBt *self = NM_DEVICE_BT (object); NMDeviceBtPrivate *priv = NM_DEVICE_BT_GET_PRIVATE (self); const char *my_hwaddr; - gs_free_error GError *error = NULL; - - priv->mm_proxy = g_dbus_proxy_new_for_bus_sync (G_BUS_TYPE_SYSTEM, - G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES | - G_DBUS_PROXY_FLAGS_DO_NOT_CONNECT_SIGNALS | - G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START, - NULL, - NM_MODEM_MANAGER_MM_DBUS_SERVICE, - NM_MODEM_MANAGER_MM_DBUS_PATH, - NM_MODEM_MANAGER_MM_DBUS_INTERFACE, - NULL, &error); - if (priv->mm_proxy) { - g_signal_connect (priv->mm_proxy, "notify::g-name-owner", - G_CALLBACK (mm_name_owner_changed_cb), - self); - mm_name_owner_changed (self); - } else { - _LOGW (LOGD_MB, "Could not create proxy for '%s': %s", - NM_MODEM_MANAGER_MM_DBUS_SERVICE, error->message); - g_clear_error (&error); - } G_OBJECT_CLASS (nm_device_bt_parent_class)->constructed (object); + priv->modem_manager = g_object_ref (nm_modem_manager_get ()); + + nm_modem_manager_name_owner_ref (priv->modem_manager); + + g_signal_connect (priv->modem_manager, + "notify::"NM_MODEM_MANAGER_NAME_OWNER, + G_CALLBACK (mm_name_owner_changed_cb), + self); + if (priv->bt_device) { /* Watch for BT device property changes */ g_signal_connect (priv->bt_device, "notify::" NM_BLUEZ_DEVICE_CONNECTED, @@ -1100,6 +1082,8 @@ constructed (GObject *object) priv->bdaddr = g_strdup (my_hwaddr); else g_warn_if_reached (); + + set_mm_running (self); } NMDevice * @@ -1137,9 +1121,10 @@ dispose (GObject *object) g_signal_handlers_disconnect_matched (priv->bt_device, G_SIGNAL_MATCH_DATA, 0, 0, NULL, NULL, object); - if (priv->mm_proxy) { - g_signal_handlers_disconnect_by_func (priv->mm_proxy, G_CALLBACK (mm_name_owner_changed_cb), object); - g_clear_object (&priv->mm_proxy); + if (priv->modem_manager) { + g_signal_handlers_disconnect_by_func (priv->modem_manager, G_CALLBACK (mm_name_owner_changed_cb), object); + nm_modem_manager_name_owner_unref (priv->modem_manager); + g_clear_object (&priv->modem_manager); } modem_cleanup (NM_DEVICE_BT (object)); diff --git a/src/devices/wwan/libnm-wwan.ver b/src/devices/wwan/libnm-wwan.ver index eb577aafb7..6efcb03fa7 100644 --- a/src/devices/wwan/libnm-wwan.ver +++ b/src/devices/wwan/libnm-wwan.ver @@ -20,7 +20,11 @@ global: nm_modem_get_type; nm_modem_get_uid; nm_modem_ip4_pre_commit; + nm_modem_manager_get; nm_modem_manager_get_type; + nm_modem_manager_name_owner_get; + nm_modem_manager_name_owner_ref; + nm_modem_manager_name_owner_unref; nm_modem_owns_port; nm_modem_set_mm_enabled; nm_modem_stage3_ip4_config_start; diff --git a/src/devices/wwan/nm-modem-manager.c b/src/devices/wwan/nm-modem-manager.c index d6a515077a..b5aa60c87a 100644 --- a/src/devices/wwan/nm-modem-manager.c +++ b/src/devices/wwan/nm-modem-manager.c @@ -45,6 +45,10 @@ /*****************************************************************************/ +NM_GOBJECT_PROPERTIES_DEFINE (NMModemManager, + PROP_NAME_OWNER, +); + enum { MODEM_ADDED, LAST_SIGNAL, @@ -65,6 +69,11 @@ typedef struct { gulong handle_object_added_id; gulong handle_object_removed_id; guint relaunch_id; + + GDBusProxy *proxy; + GCancellable *proxy_cancellable; + guint proxy_ref_count; + char *proxy_name_owner; } modm; #if WITH_OFONO @@ -435,6 +444,129 @@ modm_schedule_manager_relaunch (NMModemManager *self, /*****************************************************************************/ +static void +modm_proxy_name_owner_reset (NMModemManager *self) +{ + NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); + char *name = NULL; + + if (priv->modm.proxy) + name = g_dbus_proxy_get_name_owner (priv->modm.proxy); + + if (nm_streq0 (priv->modm.proxy_name_owner, name)) { + g_free (name); + return; + } + g_free (priv->modm.proxy_name_owner); + priv->modm.proxy_name_owner = name; + + _notify (self, PROP_NAME_OWNER); +} + +static void +modm_proxy_name_owner_changed_cb (GObject *object, + GParamSpec *pspec, + gpointer user_data) +{ + modm_proxy_name_owner_reset (user_data); +} + +static void +modm_proxy_new_cb (GObject *source_object, + GAsyncResult *result, + gpointer user_data) +{ + NMModemManager *self; + NMModemManagerPrivate *priv; + GDBusProxy *proxy; + gs_free_error GError *error = NULL; + + proxy = g_dbus_proxy_new_for_bus_finish (result, &error); + if ( !proxy + && g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + return; + + self = user_data; + priv = NM_MODEM_MANAGER_GET_PRIVATE (self); + + g_clear_object (&priv->modm.proxy_cancellable); + + if (!proxy) { + _LOGW ("could not obtain D-Bus proxy for ModemManager: %s", error->message); + return; + } + + priv->modm.proxy = proxy; + g_signal_connect (priv->modm.proxy, "notify::g-name-owner", + G_CALLBACK (modm_proxy_name_owner_changed_cb), self); + + modm_proxy_name_owner_reset (self); +} + +void +nm_modem_manager_name_owner_ref (NMModemManager *self) +{ + NMModemManagerPrivate *priv; + + g_return_if_fail (NM_IS_MODEM_MANAGER (self)); + + priv = NM_MODEM_MANAGER_GET_PRIVATE (self); + + if (priv->modm.proxy_ref_count++ > 0) { + /* only try once to create the proxy. If proxy creation + * for the first "ref" failed, it's unclear what to do. + * The proxy is hosed. */ + return; + } + + nm_assert (!priv->modm.proxy && !priv->modm.proxy_cancellable); + + priv->modm.proxy_cancellable = g_cancellable_new (); + + g_dbus_proxy_new_for_bus (G_BUS_TYPE_SYSTEM, + G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES + | G_DBUS_PROXY_FLAGS_DO_NOT_CONNECT_SIGNALS + | G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START, + NULL, + NM_MODEM_MANAGER_MM_DBUS_SERVICE, + NM_MODEM_MANAGER_MM_DBUS_PATH, + NM_MODEM_MANAGER_MM_DBUS_INTERFACE, + priv->modm.proxy_cancellable, + modm_proxy_new_cb, + self); +} + +void +nm_modem_manager_name_owner_unref (NMModemManager *self) +{ + NMModemManagerPrivate *priv; + + g_return_if_fail (NM_IS_MODEM_MANAGER (self)); + + priv = NM_MODEM_MANAGER_GET_PRIVATE (self); + + g_return_if_fail (priv->modm.proxy_ref_count > 0); + + if (--priv->modm.proxy_ref_count > 0) + return; + + nm_clear_g_cancellable (&priv->modm.proxy_cancellable); + g_clear_object (&priv->modm.proxy); + + modm_proxy_name_owner_reset (self); +} + +const char * +nm_modem_manager_name_owner_get (NMModemManager *self) +{ + g_return_val_if_fail (NM_IS_MODEM_MANAGER (self), NULL); + nm_assert (NM_MODEM_MANAGER_GET_PRIVATE (self)->modm.proxy_ref_count > 0); + + return NM_MODEM_MANAGER_GET_PRIVATE (self)->modm.proxy_name_owner; +} + +/*****************************************************************************/ + #if WITH_OFONO static void @@ -669,6 +801,25 @@ bus_get_ready (GObject *source, /*****************************************************************************/ +static void +get_property (GObject *object, guint prop_id, + GValue *value, GParamSpec *pspec) +{ + NMModemManager *self = NM_MODEM_MANAGER (object); + NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); + + switch (prop_id) { + case PROP_NAME_OWNER: + g_value_set_string (value, priv->modm.proxy_name_owner); + break; + default: + G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); + break; + } +} + +/*****************************************************************************/ + static void nm_modem_manager_init (NMModemManager *self) { @@ -695,6 +846,10 @@ dispose (GObject *object) nm_clear_g_source (&priv->modm.relaunch_id); + nm_clear_g_cancellable (&priv->modm.proxy_cancellable); + g_clear_object (&priv->modm.proxy); + nm_clear_g_free (&priv->modm.proxy_name_owner); + modm_clear_manager (self); #if WITH_OFONO @@ -723,6 +878,15 @@ nm_modem_manager_class_init (NMModemManagerClass *klass) GObjectClass *object_class = G_OBJECT_CLASS (klass); object_class->dispose = dispose; + object_class->get_property = get_property; + + obj_properties[PROP_NAME_OWNER] = + g_param_spec_string (NM_MODEM_MANAGER_NAME_OWNER, "", "", + NULL, + G_PARAM_READABLE + | G_PARAM_STATIC_STRINGS); + + g_object_class_install_properties (object_class, _PROPERTY_ENUMS_LAST, obj_properties); signals[MODEM_ADDED] = g_signal_new (NM_MODEM_MANAGER_MODEM_ADDED, diff --git a/src/devices/wwan/nm-modem-manager.h b/src/devices/wwan/nm-modem-manager.h index 84eb232e98..5f913083a9 100644 --- a/src/devices/wwan/nm-modem-manager.h +++ b/src/devices/wwan/nm-modem-manager.h @@ -34,6 +34,8 @@ #define NM_MODEM_MANAGER_MODEM_ADDED "modem-added" +#define NM_MODEM_MANAGER_NAME_OWNER "name-owner" + #define NM_MODEM_MANAGER_MM_DBUS_SERVICE "org.freedesktop.ModemManager1" #define NM_MODEM_MANAGER_MM_DBUS_PATH "/org/freedesktop/ModemManager1" #define NM_MODEM_MANAGER_MM_DBUS_INTERFACE "org.freedesktop.ModemManager1" @@ -45,4 +47,9 @@ GType nm_modem_manager_get_type (void); NMModemManager *nm_modem_manager_get (void); +void nm_modem_manager_name_owner_ref (NMModemManager *self); +void nm_modem_manager_name_owner_unref (NMModemManager *self); + +const char *nm_modem_manager_name_owner_get (NMModemManager *self); + #endif /* __NETWORKMANAGER_MODEM_MANAGER_H__ */ -- cgit v1.2.1 From 44f5e372ce8f13a341582b212aa6f3a28290166e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 22 Apr 2017 12:19:27 +0200 Subject: bt: create D-Bus proxy for NMBluez4Adapter asynchronously And some fixes: - proxy creation may fail. Don't handle it by retrying, but at least don't access the invalid proxy instance. - get_properties_cb() did not take a reference to @self nor was the operation cancellable. This leads to crash if the instance gets destroyed early. --- src/devices/bluetooth/nm-bluez4-adapter.c | 93 ++++++++++++++++++++++--------- 1 file changed, 68 insertions(+), 25 deletions(-) diff --git a/src/devices/bluetooth/nm-bluez4-adapter.c b/src/devices/bluetooth/nm-bluez4-adapter.c index c0c1be30b7..5c09582b04 100644 --- a/src/devices/bluetooth/nm-bluez4-adapter.c +++ b/src/devices/bluetooth/nm-bluez4-adapter.c @@ -49,6 +49,7 @@ static guint signals[LAST_SIGNAL] = { 0 }; typedef struct { char *path; GDBusProxy *proxy; + GCancellable *proxy_cancellable; gboolean initialized; char *address; @@ -198,19 +199,28 @@ device_removed (GDBusProxy *proxy, const char *path, gpointer user_data) static void get_properties_cb (GObject *proxy, GAsyncResult *result, gpointer user_data) { - NMBluez4Adapter *self = NM_BLUEZ4_ADAPTER (user_data); - NMBluez4AdapterPrivate *priv = NM_BLUEZ4_ADAPTER_GET_PRIVATE (self); - GError *err = NULL; + NMBluez4Adapter *self; + NMBluez4AdapterPrivate *priv; + gs_free_error GError *error = NULL; GVariant *ret, *properties; char **devices; int i; ret = _nm_dbus_proxy_call_finish (G_DBUS_PROXY (proxy), result, - G_VARIANT_TYPE ("(a{sv})"), &err); + G_VARIANT_TYPE ("(a{sv})"), &error); + + if ( !ret + && g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + return; + + self = NM_BLUEZ4_ADAPTER (user_data); + priv = NM_BLUEZ4_ADAPTER_GET_PRIVATE (self); + + g_clear_object (&priv->proxy_cancellable); + if (!ret) { - g_dbus_error_strip_remote_error (err); - nm_log_warn (LOGD_BT, "bluez error getting adapter properties: %s", err->message); - g_error_free (err); + g_dbus_error_strip_remote_error (error); + nm_log_warn (LOGD_BT, "bluez error getting adapter properties: %s", error->message); goto done; } @@ -233,15 +243,43 @@ done: } static void -query_properties (NMBluez4Adapter *self) +_proxy_new_cb (GObject *source_object, + GAsyncResult *result, + gpointer user_data) { - NMBluez4AdapterPrivate *priv = NM_BLUEZ4_ADAPTER_GET_PRIVATE (self); + NMBluez4Adapter *self; + NMBluez4AdapterPrivate *priv; + gs_free_error GError *error = NULL; + GDBusProxy *proxy; + + proxy = g_dbus_proxy_new_for_bus_finish (result, &error); + if ( !proxy + && g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + return; + + self = user_data; + priv = NM_BLUEZ4_ADAPTER_GET_PRIVATE (self); + + if (!proxy) { + nm_log_warn (LOGD_BT, "bluez error creating D-Bus proxy: %s", error->message); + g_clear_object (&priv->proxy_cancellable); + g_signal_emit (self, signals[INITIALIZED], 0, priv->initialized); + return; + } + + priv->proxy = proxy; + + _nm_dbus_signal_connect (priv->proxy, "DeviceCreated", G_VARIANT_TYPE ("(o)"), + G_CALLBACK (device_created), self); + _nm_dbus_signal_connect (priv->proxy, "DeviceRemoved", G_VARIANT_TYPE ("(o)"), + G_CALLBACK (device_removed), self); g_dbus_proxy_call (priv->proxy, "GetProperties", NULL, G_DBUS_CALL_FLAGS_NONE, -1, - NULL, - get_properties_cb, self); + priv->proxy_cancellable, + get_properties_cb, + self); } /*****************************************************************************/ @@ -316,19 +354,17 @@ nm_bluez4_adapter_new (const char *path, NMSettings *settings) priv->settings = g_object_ref (settings); - priv->proxy = g_dbus_proxy_new_for_bus_sync (G_BUS_TYPE_SYSTEM, - G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES, - NULL, - BLUEZ_SERVICE, - priv->path, - BLUEZ4_ADAPTER_INTERFACE, - NULL, NULL); - _nm_dbus_signal_connect (priv->proxy, "DeviceCreated", G_VARIANT_TYPE ("(o)"), - G_CALLBACK (device_created), self); - _nm_dbus_signal_connect (priv->proxy, "DeviceRemoved", G_VARIANT_TYPE ("(o)"), - G_CALLBACK (device_removed), self); + priv->proxy_cancellable = g_cancellable_new (); - query_properties (self); + g_dbus_proxy_new_for_bus (G_BUS_TYPE_SYSTEM, + G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES, + NULL, + BLUEZ_SERVICE, + priv->path, + BLUEZ4_ADAPTER_INTERFACE, + priv->proxy_cancellable, + _proxy_new_cb, + self); return self; } @@ -339,21 +375,28 @@ dispose (GObject *object) NMBluez4AdapterPrivate *priv = NM_BLUEZ4_ADAPTER_GET_PRIVATE (self); NMBluezDevice *device; + nm_clear_g_cancellable (&priv->proxy_cancellable); + while ((device = g_hash_table_find (priv->devices, _find_all, NULL))) device_do_remove (self, device); + if (priv->proxy) { + g_signal_handlers_disconnect_by_data (priv->proxy, self); + g_clear_object (&priv->proxy); + } + G_OBJECT_CLASS (nm_bluez4_adapter_parent_class)->dispose (object); } static void finalize (GObject *object) { - NMBluez4AdapterPrivate *priv = NM_BLUEZ4_ADAPTER_GET_PRIVATE ((NMBluez4Adapter *) object); + NMBluez4Adapter *self = NM_BLUEZ4_ADAPTER (object); + NMBluez4AdapterPrivate *priv = NM_BLUEZ4_ADAPTER_GET_PRIVATE (self); g_hash_table_destroy (priv->devices); g_free (priv->address); g_free (priv->path); - g_object_unref (priv->proxy); G_OBJECT_CLASS (nm_bluez4_adapter_parent_class)->finalize (object); -- cgit v1.2.1 From e68a33d1148b1135bfa77c333a46f6a0355c3654 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 22 Apr 2017 20:06:29 +0200 Subject: bt: create D-Bus proxy for NMBluez4Manager asynchronously And some fixes: - proxy creation may fail. Don't handle it by retrying, but at least don't access the invalid proxy instance. - quering "DefaultAdapter" did not take a reference to @self nor was the operation cancellable. This leads to crash if the instance gets destroyed early. --- src/devices/bluetooth/nm-bluez4-manager.c | 138 ++++++++++++++++++++---------- 1 file changed, 93 insertions(+), 45 deletions(-) diff --git a/src/devices/bluetooth/nm-bluez4-manager.c b/src/devices/bluetooth/nm-bluez4-manager.c index a9079a2fc0..3b233fcdb5 100644 --- a/src/devices/bluetooth/nm-bluez4-manager.c +++ b/src/devices/bluetooth/nm-bluez4-manager.c @@ -47,6 +47,7 @@ typedef struct { NMSettings *settings; GDBusProxy *proxy; + GCancellable *proxy_cancellable; NMBluez4Adapter *adapter; } NMBluez4ManagerPrivate; @@ -176,42 +177,62 @@ default_adapter_changed (GDBusProxy *proxy, const char *path, NMBluez4Manager *s static void default_adapter_cb (GObject *proxy, GAsyncResult *result, gpointer user_data) { - NMBluez4Manager *self = NM_BLUEZ4_MANAGER (user_data); - NMBluez4ManagerPrivate *priv = NM_BLUEZ4_MANAGER_GET_PRIVATE (self); - GVariant *ret; - GError *err = NULL; + NMBluez4Manager *self; + NMBluez4ManagerPrivate *priv; + gs_unref_variant GVariant *ret = NULL; + gs_free_error GError *error = NULL; + const char *default_adapter; ret = _nm_dbus_proxy_call_finish (G_DBUS_PROXY (proxy), result, - G_VARIANT_TYPE ("(o)"), &err); - if (ret) { - const char *default_adapter; + G_VARIANT_TYPE ("(o)"), &error); + if ( !ret + && g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + return; - g_variant_get (ret, "(&o)", &default_adapter); - default_adapter_changed (priv->proxy, default_adapter, self); - g_variant_unref (ret); - } else { + self = NM_BLUEZ4_MANAGER (user_data); + priv = NM_BLUEZ4_MANAGER_GET_PRIVATE (self); + + g_clear_object (&priv->proxy_cancellable); + + if (!ret) { /* Ignore "No such adapter" errors; just means bluetooth isn't active */ - if ( !_nm_dbus_error_has_name (err, "org.bluez.Error.NoSuchAdapter") - && !_nm_dbus_error_has_name (err, "org.freedesktop.systemd1.LoadFailed") - && !g_error_matches (err, G_DBUS_ERROR, G_DBUS_ERROR_SERVICE_UNKNOWN)) { - g_dbus_error_strip_remote_error (err); + if ( !_nm_dbus_error_has_name (error, "org.bluez.Error.NoSuchAdapter") + && !_nm_dbus_error_has_name (error, "org.freedesktop.systemd1.LoadFailed") + && !g_error_matches (error, G_DBUS_ERROR, G_DBUS_ERROR_SERVICE_UNKNOWN)) { + g_dbus_error_strip_remote_error (error); nm_log_warn (LOGD_BT, "bluez error getting default adapter: %s", - err->message); + error->message); } - g_error_free (err); + return; } + + g_variant_get (ret, "(&o)", &default_adapter); + default_adapter_changed (priv->proxy, default_adapter, self); } static void -query_default_adapter (NMBluez4Manager *self) +name_owner_changed (NMBluez4Manager *self) { NMBluez4ManagerPrivate *priv = NM_BLUEZ4_MANAGER_GET_PRIVATE (self); + gs_free char *owner = NULL; + + nm_clear_g_cancellable (&priv->proxy_cancellable); + + owner = g_dbus_proxy_get_name_owner (priv->proxy); + if (!owner) { + /* Throwing away the adapter removes all devices too */ + g_clear_object (&priv->adapter); + return; + } + + priv->proxy_cancellable = g_cancellable_new (); g_dbus_proxy_call (priv->proxy, "DefaultAdapter", NULL, G_DBUS_CALL_FLAGS_NONE, -1, - NULL, - default_adapter_cb, self); + priv->proxy_cancellable, + default_adapter_cb, + self); } static void @@ -219,34 +240,35 @@ name_owner_changed_cb (GObject *object, GParamSpec *pspec, gpointer user_data) { - NMBluez4Manager *self = NM_BLUEZ4_MANAGER (user_data); - NMBluez4ManagerPrivate *priv = NM_BLUEZ4_MANAGER_GET_PRIVATE (self); - char *owner; - - owner = g_dbus_proxy_get_name_owner (priv->proxy); - if (owner) { - query_default_adapter (self); - g_free (owner); - } else { - /* Throwing away the adapter removes all devices too */ - g_clear_object (&priv->adapter); - } + name_owner_changed (user_data); } -/*****************************************************************************/ - static void -nm_bluez4_manager_init (NMBluez4Manager *self) +_proxy_new_cb (GObject *source_object, + GAsyncResult *result, + gpointer user_data) { - NMBluez4ManagerPrivate *priv = NM_BLUEZ4_MANAGER_GET_PRIVATE (self); + NMBluez4Manager *self; + NMBluez4ManagerPrivate *priv; + gs_free_error GError *error = NULL; + GDBusProxy *proxy; + + proxy = g_dbus_proxy_new_for_bus_finish (result, &error); + if ( !proxy + && g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + return; + + self = user_data; + priv = NM_BLUEZ4_MANAGER_GET_PRIVATE (self); + + if (!proxy) { + nm_log_warn (LOGD_BT, "bluez error creating D-Bus proxy: %s", error->message); + g_clear_object (&priv->proxy_cancellable); + return; + } + + priv->proxy = proxy; - priv->proxy = g_dbus_proxy_new_for_bus_sync (G_BUS_TYPE_SYSTEM, - G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES, - NULL, - BLUEZ_SERVICE, - BLUEZ_MANAGER_PATH, - BLUEZ4_MANAGER_INTERFACE, - NULL, NULL); _nm_dbus_signal_connect (priv->proxy, "AdapterRemoved", G_VARIANT_TYPE ("(o)"), G_CALLBACK (adapter_removed), self); _nm_dbus_signal_connect (priv->proxy, "DefaultAdapterChanged", G_VARIANT_TYPE ("(o)"), @@ -254,7 +276,27 @@ nm_bluez4_manager_init (NMBluez4Manager *self) g_signal_connect (priv->proxy, "notify::g-name-owner", G_CALLBACK (name_owner_changed_cb), self); - query_default_adapter (self); + name_owner_changed (self); +} + +/*****************************************************************************/ + +static void +nm_bluez4_manager_init (NMBluez4Manager *self) +{ + NMBluez4ManagerPrivate *priv = NM_BLUEZ4_MANAGER_GET_PRIVATE (self); + + priv->proxy_cancellable = g_cancellable_new (); + + g_dbus_proxy_new_for_bus (G_BUS_TYPE_SYSTEM, + G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES, + NULL, + BLUEZ_SERVICE, + BLUEZ_MANAGER_PATH, + BLUEZ4_MANAGER_INTERFACE, + priv->proxy_cancellable, + _proxy_new_cb, + self); } NMBluez4Manager * @@ -275,7 +317,13 @@ dispose (GObject *object) NMBluez4Manager *self = NM_BLUEZ4_MANAGER (object); NMBluez4ManagerPrivate *priv = NM_BLUEZ4_MANAGER_GET_PRIVATE (self); - g_clear_object (&priv->proxy); + nm_clear_g_cancellable (&priv->proxy_cancellable); + + if (priv->proxy) { + g_signal_handlers_disconnect_by_data (priv->proxy, self); + g_clear_object (&priv->proxy); + } + g_clear_object (&priv->adapter); G_OBJECT_CLASS (nm_bluez4_manager_parent_class)->dispose (object); -- cgit v1.2.1 From 7d11fe55819b998fb168b4fe7eb94f0cc31091b6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 22 Apr 2017 20:11:20 +0200 Subject: bt: use logging macros in nm-bluez4-adapter.c --- src/devices/bluetooth/nm-bluez4-adapter.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/src/devices/bluetooth/nm-bluez4-adapter.c b/src/devices/bluetooth/nm-bluez4-adapter.c index 5c09582b04..cd9eaed34a 100644 --- a/src/devices/bluetooth/nm-bluez4-adapter.c +++ b/src/devices/bluetooth/nm-bluez4-adapter.c @@ -74,6 +74,11 @@ G_DEFINE_TYPE (NMBluez4Adapter, nm_bluez4_adapter, G_TYPE_OBJECT) /*****************************************************************************/ +#define _NMLOG_DOMAIN LOGD_BT +#define _NMLOG(level, ...) __NMLOG_DEFAULT (level, _NMLOG_DOMAIN, "bluez4-adapter", __VA_ARGS__) + +/*****************************************************************************/ + static void device_do_remove (NMBluez4Adapter *self, NMBluezDevice *device); /*****************************************************************************/ @@ -120,8 +125,8 @@ nm_bluez4_adapter_get_devices (NMBluez4Adapter *self) static void emit_device_removed (NMBluez4Adapter *self, NMBluezDevice *device) { - nm_log_dbg (LOGD_BT, "(%s): bluez device now unusable", - nm_bluez_device_get_path (device)); + _LOGD ("(%s): bluez device now unusable", + nm_bluez_device_get_path (device)); g_signal_emit (self, signals[DEVICE_REMOVED], 0, device); } @@ -131,9 +136,9 @@ device_usable (NMBluezDevice *device, GParamSpec *pspec, gpointer user_data) NMBluez4Adapter *self = NM_BLUEZ4_ADAPTER (user_data); if (nm_bluez_device_get_usable (device)) { - nm_log_dbg (LOGD_BT, "(%s): bluez device now usable (device address is %s)", - nm_bluez_device_get_path (device), - nm_bluez_device_get_address (device)); + _LOGD ("(%s): bluez device now usable (device address is %s)", + nm_bluez_device_get_path (device), + nm_bluez_device_get_address (device)); g_signal_emit (self, signals[DEVICE_ADDED], 0, device); } else emit_device_removed (self, device); @@ -144,9 +149,9 @@ device_initialized (NMBluezDevice *device, gboolean success, gpointer user_data) { NMBluez4Adapter *self = NM_BLUEZ4_ADAPTER (user_data); - nm_log_dbg (LOGD_BT, "(%s): bluez device %s", - nm_bluez_device_get_path (device), - success ? "initialized" : "failed to initialize"); + _LOGD ("(%s): bluez device %s", + nm_bluez_device_get_path (device), + success ? "initialized" : "failed to initialize"); if (!success) device_do_remove (self, device); } @@ -179,7 +184,7 @@ device_created (GDBusProxy *proxy, const char *path, gpointer user_data) g_signal_connect (device, "notify::usable", G_CALLBACK (device_usable), self); g_hash_table_insert (priv->devices, (gpointer) nm_bluez_device_get_path (device), device); - nm_log_dbg (LOGD_BT, "(%s): new bluez device found", path); + _LOGD ("(%s): new bluez device found", path); } static void @@ -189,7 +194,7 @@ device_removed (GDBusProxy *proxy, const char *path, gpointer user_data) NMBluez4AdapterPrivate *priv = NM_BLUEZ4_ADAPTER_GET_PRIVATE (self); NMBluezDevice *device; - nm_log_dbg (LOGD_BT, "(%s): bluez device removed", path); + _LOGD ("(%s): bluez device removed", path); device = g_hash_table_lookup (priv->devices, path); if (device) @@ -220,7 +225,7 @@ get_properties_cb (GObject *proxy, GAsyncResult *result, gpointer user_data) if (!ret) { g_dbus_error_strip_remote_error (error); - nm_log_warn (LOGD_BT, "bluez error getting adapter properties: %s", error->message); + _LOGW ("bluez error getting adapter properties: %s", error->message); goto done; } @@ -261,7 +266,7 @@ _proxy_new_cb (GObject *source_object, priv = NM_BLUEZ4_ADAPTER_GET_PRIVATE (self); if (!proxy) { - nm_log_warn (LOGD_BT, "bluez error creating D-Bus proxy: %s", error->message); + _LOGW ("bluez error creating D-Bus proxy: %s", error->message); g_clear_object (&priv->proxy_cancellable); g_signal_emit (self, signals[INITIALIZED], 0, priv->initialized); return; -- cgit v1.2.1 From f61b1dadaee208967c765a2ca235a4b8dbd9f291 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 22 Apr 2017 20:13:42 +0200 Subject: bt: use logging macros in nm-bluez4-manager.c --- src/devices/bluetooth/nm-bluez4-manager.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/devices/bluetooth/nm-bluez4-manager.c b/src/devices/bluetooth/nm-bluez4-manager.c index 3b233fcdb5..66ef83104d 100644 --- a/src/devices/bluetooth/nm-bluez4-manager.c +++ b/src/devices/bluetooth/nm-bluez4-manager.c @@ -67,6 +67,11 @@ G_DEFINE_TYPE (NMBluez4Manager, nm_bluez4_manager, G_TYPE_OBJECT) /*****************************************************************************/ +#define _NMLOG_DOMAIN LOGD_BT +#define _NMLOG(level, ...) __NMLOG_DEFAULT (level, _NMLOG_DOMAIN, "bluez4-manager", __VA_ARGS__) + +/*****************************************************************************/ + static void emit_bdaddr_added (NMBluez4Manager *self, NMBluezDevice *device) { @@ -200,8 +205,8 @@ default_adapter_cb (GObject *proxy, GAsyncResult *result, gpointer user_data) && !_nm_dbus_error_has_name (error, "org.freedesktop.systemd1.LoadFailed") && !g_error_matches (error, G_DBUS_ERROR, G_DBUS_ERROR_SERVICE_UNKNOWN)) { g_dbus_error_strip_remote_error (error); - nm_log_warn (LOGD_BT, "bluez error getting default adapter: %s", - error->message); + _LOGW ("bluez error getting default adapter: %s", + error->message); } return; } @@ -262,7 +267,7 @@ _proxy_new_cb (GObject *source_object, priv = NM_BLUEZ4_MANAGER_GET_PRIVATE (self); if (!proxy) { - nm_log_warn (LOGD_BT, "bluez error creating D-Bus proxy: %s", error->message); + _LOGW ("bluez error creating D-Bus proxy: %s", error->message); g_clear_object (&priv->proxy_cancellable); return; } -- cgit v1.2.1 From 615aa3f0777e9421ccaf888a44d98d968737bba8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 22 Apr 2017 21:06:12 +0200 Subject: modem: prettify logging output about available ModemManager/oFono These lines are logged with level. They should look pleasant. --- src/devices/wwan/nm-modem-manager.c | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/src/devices/wwan/nm-modem-manager.c b/src/devices/wwan/nm-modem-manager.c index b5aa60c87a..3dd9377699 100644 --- a/src/devices/wwan/nm-modem-manager.c +++ b/src/devices/wwan/nm-modem-manager.c @@ -70,6 +70,15 @@ typedef struct { gulong handle_object_removed_id; guint relaunch_id; + /* this only has one use: that the logging line about + * ModemManager available distinguishes between first-time + * and later name-owner-changed. */ + enum { + LOG_AVAILABLE_NOT_INITIALIZED = 0, + LOG_AVAILABLE_YES, + LOG_AVAILABLE_NO, + } log_available:3; + GDBusProxy *proxy; GCancellable *proxy_cancellable; guint proxy_ref_count; @@ -219,7 +228,10 @@ modm_manager_available (NMModemManager *self) NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); GList *modems, *l; - _LOGI ("ModemManager available in the bus"); + if (priv->modm.log_available != LOG_AVAILABLE_YES) { + _LOGI ("ModemManager %savailable", priv->modm.log_available ? "now " : ""); + priv->modm.log_available = LOG_AVAILABLE_YES; + } /* Update initial modems list */ modems = g_dbus_object_manager_get_objects (G_DBUS_OBJECT_MANAGER (priv->modm.manager)); @@ -241,7 +253,10 @@ modm_handle_name_owner_changed (MMManager *modem_manager, name_owner = g_dbus_object_manager_client_get_name_owner (G_DBUS_OBJECT_MANAGER_CLIENT (modem_manager)); if (!name_owner) { - _LOGI ("ModemManager disappeared from bus"); + if (priv->modm.log_available != LOG_AVAILABLE_NO) { + _LOGI ("ModemManager %savailable", priv->modm.log_available ? "no longer " : "not "); + priv->modm.log_available = LOG_AVAILABLE_NO; + } /* If not managed by systemd, schedule relaunch */ if (!sd_booted ()) @@ -659,14 +674,14 @@ ofono_enumerate_devices_done (GObject *proxy, } static void -ofono_check_name_owner (NMModemManager *self) +ofono_check_name_owner (NMModemManager *self, gboolean first_invocation) { NMModemManagerPrivate *priv = NM_MODEM_MANAGER_GET_PRIVATE (self); gs_free char *name_owner = NULL; name_owner = g_dbus_proxy_get_name_owner (G_DBUS_PROXY (priv->ofono.proxy)); if (name_owner) { - _LOGI ("oFono is now available"); + _LOGI ("oFono is %savailable", first_invocation ? "" : "now "); nm_clear_g_cancellable (&priv->ofono.cancellable); priv->ofono.cancellable = g_cancellable_new (); @@ -683,7 +698,7 @@ ofono_check_name_owner (NMModemManager *self) GHashTableIter iter; NMModem *modem; - _LOGI ("oFono disappeared from bus"); + _LOGI ("oFono is %savailable", first_invocation ? "not " : "no longer "); /* Remove any oFono modems that might be left around */ g_hash_table_iter_init (&iter, priv->modems); @@ -701,7 +716,7 @@ ofono_name_owner_changed (GDBusProxy *ofono_proxy, GParamSpec *pspec, NMModemManager *self) { - ofono_check_name_owner (self); + ofono_check_name_owner (self, FALSE); } static void @@ -741,7 +756,7 @@ ofono_proxy_new_cb (GObject *source_object, G_CALLBACK (ofono_signal_cb), self); - ofono_check_name_owner (self); + ofono_check_name_owner (self, TRUE); } static void -- cgit v1.2.1 From b7329fccceead7c4a3997e649835719c6bde3166 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 22 Apr 2017 21:56:59 +0200 Subject: ofono: take D-Bus proxy for SimManager asynchronously Also, - fix memleaks in sim_get_properties_done() - don't take reference to self for GetProperties call --- src/devices/wwan/nm-modem-ofono.c | 130 +++++++++++++++++++++++++------------- 1 file changed, 85 insertions(+), 45 deletions(-) diff --git a/src/devices/wwan/nm-modem-ofono.c b/src/devices/wwan/nm-modem-ofono.c index 714f9e8348..d29b45b6ab 100644 --- a/src/devices/wwan/nm-modem-ofono.c +++ b/src/devices/wwan/nm-modem-ofono.c @@ -46,6 +46,8 @@ typedef struct { GDBusProxy *context_proxy; GDBusProxy *sim_proxy; + GCancellable *sim_proxy_cancellable; + GError *property_error; char *context_path; @@ -375,22 +377,35 @@ sim_property_changed (GDBusProxy *proxy, } static void -sim_get_properties_done (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data) +sim_get_properties_done (GObject *source, + GAsyncResult *result, + gpointer user_data) { - gs_unref_object NMModemOfono *self = NM_MODEM_OFONO (user_data); - GError *error = NULL; - GVariant *v_properties, *v_dict, *v; + NMModemOfono *self; + NMModemOfonoPrivate *priv; + gs_free_error GError *error = NULL; + gs_unref_variant GVariant *v_properties = NULL; + gs_unref_variant GVariant *v_dict = NULL; + GVariant *v; GVariantIter i; const char *property; - v_properties = _nm_dbus_proxy_call_finish (proxy, + v_properties = _nm_dbus_proxy_call_finish (G_DBUS_PROXY (source), result, G_VARIANT_TYPE ("(a{sv})"), &error); + if ( !v_properties + && g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + return; + + self = NM_MODEM_OFONO (user_data); + priv = NM_MODEM_OFONO_GET_PRIVATE (self); + + g_clear_object (&priv->sim_proxy_cancellable); + if (!v_properties) { g_dbus_error_strip_remote_error (error); _LOGW ("error getting sim properties: %s", error->message); - g_error_free (error); return; } @@ -418,9 +433,49 @@ sim_get_properties_done (GDBusProxy *proxy, GAsyncResult *result, gpointer user_ handle_sim_property (NULL, property, v, self); g_variant_unref (v); } +} - g_variant_unref (v_dict); - g_variant_unref (v_properties); +static void +_sim_proxy_new_cb (GObject *source, + GAsyncResult *result, + gpointer user_data) +{ + NMModemOfono *self; + NMModemOfonoPrivate *priv; + gs_free_error GError *error = NULL; + GDBusProxy *proxy; + + proxy = g_dbus_proxy_new_for_bus_finish (result, &error); + if ( !proxy + && g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + return; + + self = user_data; + priv = NM_MODEM_OFONO_GET_PRIVATE (self); + + if (!proxy) { + _LOGW ("failed to create SimManager proxy: %s", error->message); + g_clear_object (&priv->sim_proxy_cancellable); + return; + } + + priv->sim_proxy = proxy; + + /* Watch for custom ofono PropertyChanged signals */ + _nm_dbus_signal_connect (priv->sim_proxy, + "PropertyChanged", + G_VARIANT_TYPE ("(sv)"), + G_CALLBACK (sim_property_changed), + self); + + g_dbus_proxy_call (priv->sim_proxy, + "GetProperties", + NULL, + G_DBUS_CALL_FLAGS_NONE, + 20000, + priv->sim_proxy_cancellable, + sim_get_properties_done, + self); } static void @@ -430,47 +485,30 @@ handle_sim_iface (NMModemOfono *self, gboolean found) _LOGD ("SimManager interface %sfound", found ? "" : "not "); - if (!found && priv->sim_proxy) { + if (!found && (priv->sim_proxy || priv->sim_proxy_cancellable)) { _LOGI ("SimManager interface disappeared"); - g_signal_handlers_disconnect_by_data (priv->sim_proxy, NM_MODEM_OFONO (self)); - g_clear_object (&priv->sim_proxy); + nm_clear_g_cancellable (&priv->sim_proxy_cancellable); + if (priv->sim_proxy) { + g_signal_handlers_disconnect_by_data (priv->sim_proxy, self); + g_clear_object (&priv->sim_proxy); + } g_clear_pointer (&priv->imsi, g_free); update_modem_state (self); - } else if (found && !priv->sim_proxy) { - GError *error = NULL; - + } else if (found && (!priv->sim_proxy && !priv->sim_proxy_cancellable)) { _LOGI ("found new SimManager interface"); - priv->sim_proxy = g_dbus_proxy_new_for_bus_sync (G_BUS_TYPE_SYSTEM, - G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES - | G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START, - NULL, /* GDBusInterfaceInfo */ - OFONO_DBUS_SERVICE, - nm_modem_get_path (NM_MODEM (self)), - OFONO_DBUS_INTERFACE_SIM_MANAGER, - NULL, /* GCancellable */ - &error); - if (priv->sim_proxy == NULL) { - _LOGW ("failed to create SimManager proxy: %s", error->message); - g_error_free (error); - return; - } - - /* Watch for custom ofono PropertyChanged signals */ - _nm_dbus_signal_connect (priv->sim_proxy, - "PropertyChanged", - G_VARIANT_TYPE ("(sv)"), - G_CALLBACK (sim_property_changed), - self); - - g_dbus_proxy_call (priv->sim_proxy, - "GetProperties", - NULL, - G_DBUS_CALL_FLAGS_NONE, - 20000, - NULL, - (GAsyncReadyCallback) sim_get_properties_done, - g_object_ref (self)); + priv->sim_proxy_cancellable = g_cancellable_new (); + + g_dbus_proxy_new_for_bus (G_BUS_TYPE_SYSTEM, + G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES + | G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START, + NULL, /* GDBusInterfaceInfo */ + OFONO_DBUS_SERVICE, + nm_modem_get_path (NM_MODEM (self)), + OFONO_DBUS_INTERFACE_SIM_MANAGER, + priv->sim_proxy_cancellable, /* GCancellable */ + _sim_proxy_new_cb, + self); } } @@ -1163,6 +1201,8 @@ dispose (GObject *object) NMModemOfono *self = NM_MODEM_OFONO (object); NMModemOfonoPrivate *priv = NM_MODEM_OFONO_GET_PRIVATE (self); + nm_clear_g_cancellable (&priv->sim_proxy_cancellable); + if (priv->connect_properties) { g_hash_table_destroy (priv->connect_properties); priv->connect_properties = NULL; @@ -1179,7 +1219,7 @@ dispose (GObject *object) g_clear_object (&priv->context_proxy); if (priv->sim_proxy) { - g_signal_handlers_disconnect_by_data (priv->sim_proxy, NM_MODEM_OFONO (self)); + g_signal_handlers_disconnect_by_data (priv->sim_proxy, self); g_clear_object (&priv->sim_proxy); } -- cgit v1.2.1 From 58712c95464ad6b4bbad08411ae2670e7994445c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 22 Apr 2017 21:56:59 +0200 Subject: ofono: take D-Bus proxy for ConnectionManager asynchronously Also, - disconnect signals from connman_proxy in dispose() - don't take reference to self for GetProperties call --- src/devices/wwan/nm-modem-ofono.c | 136 ++++++++++++++++++++++++-------------- 1 file changed, 88 insertions(+), 48 deletions(-) diff --git a/src/devices/wwan/nm-modem-ofono.c b/src/devices/wwan/nm-modem-ofono.c index d29b45b6ab..98b5c53c99 100644 --- a/src/devices/wwan/nm-modem-ofono.c +++ b/src/devices/wwan/nm-modem-ofono.c @@ -46,6 +46,7 @@ typedef struct { GDBusProxy *context_proxy; GDBusProxy *sim_proxy; + GCancellable *connman_proxy_cancellable; GCancellable *sim_proxy_cancellable; GError *property_error; @@ -552,22 +553,35 @@ connman_property_changed (GDBusProxy *proxy, } static void -connman_get_properties_done (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data) +connman_get_properties_done (GObject *source, + GAsyncResult *result, + gpointer user_data) { - gs_unref_object NMModemOfono *self = NM_MODEM_OFONO (user_data); - GError *error = NULL; - GVariant *v_properties, *v_dict, *v; + NMModemOfono *self; + NMModemOfonoPrivate *priv; + gs_free_error GError *error = NULL; + gs_unref_variant GVariant *v_properties = NULL; + gs_unref_variant GVariant *v_dict = NULL; + GVariant *v; GVariantIter i; const char *property; - v_properties = _nm_dbus_proxy_call_finish (proxy, - result, - G_VARIANT_TYPE ("(a{sv})"), - &error); + v_properties = _nm_dbus_proxy_call_finish (G_DBUS_PROXY (source), + result, + G_VARIANT_TYPE ("(a{sv})"), + &error); + if ( !v_properties + && g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + return; + + self = NM_MODEM_OFONO (user_data); + priv = NM_MODEM_OFONO_GET_PRIVATE (self); + + g_clear_object (&priv->connman_proxy_cancellable); + if (!v_properties) { g_dbus_error_strip_remote_error (error); _LOGW ("error getting connman properties: %s", error->message); - g_error_free (error); return; } @@ -587,9 +601,48 @@ connman_get_properties_done (GDBusProxy *proxy, GAsyncResult *result, gpointer u handle_connman_property (NULL, property, v, self); g_variant_unref (v); } +} - g_variant_unref (v_dict); - g_variant_unref (v_properties); +static void +_connman_proxy_new_cb (GObject *source, + GAsyncResult *result, + gpointer user_data) +{ + NMModemOfono *self; + NMModemOfonoPrivate *priv; + gs_free_error GError *error = NULL; + GDBusProxy *proxy; + + proxy = g_dbus_proxy_new_for_bus_finish (result, &error); + if ( !proxy + && g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + return; + + self = user_data; + priv = NM_MODEM_OFONO_GET_PRIVATE (self); + + if (!proxy) { + _LOGW ("failed to create ConnectionManager proxy: %s", error->message); + g_clear_object (&priv->connman_proxy_cancellable); + return; + } + + priv->connman_proxy = proxy; + + _nm_dbus_signal_connect (priv->connman_proxy, + "PropertyChanged", + G_VARIANT_TYPE ("(sv)"), + G_CALLBACK (connman_property_changed), + self); + + g_dbus_proxy_call (priv->connman_proxy, + "GetProperties", + NULL, + G_DBUS_CALL_FLAGS_NONE, + 20000, + priv->connman_proxy_cancellable, + connman_get_properties_done, + self); } static void @@ -599,11 +652,13 @@ handle_connman_iface (NMModemOfono *self, gboolean found) _LOGD ("ConnectionManager interface %sfound", found ? "" : "not "); - if (!found && priv->connman_proxy) { + if (!found && (priv->connman_proxy || priv->connman_proxy_cancellable)) { _LOGI ("ConnectionManager interface disappeared"); - - g_signal_handlers_disconnect_by_data (priv->connman_proxy, NM_MODEM_OFONO (self)); - g_clear_object (&priv->connman_proxy); + nm_clear_g_cancellable (&priv->connman_proxy_cancellable); + if (priv->connman_proxy) { + g_signal_handlers_disconnect_by_data (priv->connman_proxy, self); + g_clear_object (&priv->connman_proxy); + } /* The connection manager proxy disappeared, we should * consider the modem disabled. @@ -611,41 +666,21 @@ handle_connman_iface (NMModemOfono *self, gboolean found) priv->gprs_attached = FALSE; update_modem_state (self); - } else if (found && !priv->connman_proxy) { - GError *error = NULL; - + } else if (found && (!priv->connman_proxy && !priv->connman_proxy_cancellable)) { _LOGI ("found new ConnectionManager interface"); - priv->connman_proxy = g_dbus_proxy_new_for_bus_sync (G_BUS_TYPE_SYSTEM, - G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES - | G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START, - NULL, /* GDBusInterfaceInfo */ - OFONO_DBUS_SERVICE, - nm_modem_get_path (NM_MODEM (self)), - OFONO_DBUS_INTERFACE_CONNECTION_MANAGER, - NULL, /* GCancellable */ - &error); - if (priv->connman_proxy == NULL) { - _LOGW ("failed to create ConnectionManager proxy: %s", error->message); - g_error_free (error); - return; - } + priv->connman_proxy_cancellable = g_cancellable_new (); - /* Watch for custom ofono PropertyChanged signals */ - _nm_dbus_signal_connect (priv->connman_proxy, - "PropertyChanged", - G_VARIANT_TYPE ("(sv)"), - G_CALLBACK (connman_property_changed), - self); - - g_dbus_proxy_call (priv->connman_proxy, - "GetProperties", - NULL, - G_DBUS_CALL_FLAGS_NONE, - 20000, - NULL, - (GAsyncReadyCallback) connman_get_properties_done, - g_object_ref (self)); + g_dbus_proxy_new_for_bus (G_BUS_TYPE_SYSTEM, + G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES + | G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START, + NULL, /* GDBusInterfaceInfo */ + OFONO_DBUS_SERVICE, + nm_modem_get_path (NM_MODEM (self)), + OFONO_DBUS_INTERFACE_CONNECTION_MANAGER, + priv->connman_proxy_cancellable, + _connman_proxy_new_cb, + NULL); } } @@ -1201,6 +1236,7 @@ dispose (GObject *object) NMModemOfono *self = NM_MODEM_OFONO (object); NMModemOfonoPrivate *priv = NM_MODEM_OFONO_GET_PRIVATE (self); + nm_clear_g_cancellable (&priv->connman_proxy_cancellable); nm_clear_g_cancellable (&priv->sim_proxy_cancellable); if (priv->connect_properties) { @@ -1215,7 +1251,11 @@ dispose (GObject *object) g_clear_object (&priv->modem_proxy); } - g_clear_object (&priv->connman_proxy); + if (priv->connman_proxy) { + g_signal_handlers_disconnect_by_data (priv->connman_proxy, self); + g_clear_object (&priv->connman_proxy); + } + g_clear_object (&priv->context_proxy); if (priv->sim_proxy) { -- cgit v1.2.1 From 1e5be78eb9688320f0f462c491a3e89d16adbe0e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 22 Apr 2017 22:20:38 +0200 Subject: ofono: make asynchrounous operations for modem_proxy cancellable Also, - chain up the constructed() function - fix memleak in modem_get_properties_done() --- src/devices/wwan/nm-modem-ofono.c | 73 +++++++++++++++++++++++++++------------ 1 file changed, 50 insertions(+), 23 deletions(-) diff --git a/src/devices/wwan/nm-modem-ofono.c b/src/devices/wwan/nm-modem-ofono.c index 98b5c53c99..994f13f70e 100644 --- a/src/devices/wwan/nm-modem-ofono.c +++ b/src/devices/wwan/nm-modem-ofono.c @@ -46,6 +46,7 @@ typedef struct { GDBusProxy *context_proxy; GDBusProxy *sim_proxy; + GCancellable *modem_proxy_cancellable; GCancellable *connman_proxy_cancellable; GCancellable *sim_proxy_cancellable; @@ -740,22 +741,35 @@ modem_property_changed (GDBusProxy *proxy, } static void -modem_get_properties_done (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data) +modem_get_properties_done (GObject *source, + GAsyncResult *result, + gpointer user_data) { - gs_unref_object NMModemOfono *self = NM_MODEM_OFONO (user_data); - GError *error = NULL; - GVariant *v_properties, *v_dict, *v; + NMModemOfono *self; + NMModemOfonoPrivate *priv; + gs_free_error GError *error = NULL; + gs_unref_variant GVariant *v_properties = NULL; + gs_unref_variant GVariant *v_dict = NULL; + GVariant *v; GVariantIter i; const char *property; - v_properties = _nm_dbus_proxy_call_finish (proxy, + v_properties = _nm_dbus_proxy_call_finish (G_DBUS_PROXY (source), result, G_VARIANT_TYPE ("(a{sv})"), &error); + if ( !v_properties + && g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + return; + + self = NM_MODEM_OFONO (user_data); + priv = NM_MODEM_OFONO_GET_PRIVATE (self); + + g_clear_object (&priv->modem_proxy_cancellable); + if (!v_properties) { g_dbus_error_strip_remote_error (error); _LOGW ("error getting modem properties: %s", error->message); - g_error_free (error); return; } @@ -779,9 +793,6 @@ modem_get_properties_done (GDBusProxy *proxy, GAsyncResult *result, gpointer use handle_modem_property (NULL, property, v, self); g_variant_unref (v); } - - g_variant_unref (v_dict); - g_variant_unref (v_properties); } static void @@ -1154,19 +1165,29 @@ act_stage1_prepare (NMModem *modem, } static void -modem_proxy_new_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data) +modem_proxy_new_cb (GObject *source, GAsyncResult *result, gpointer user_data) { - gs_unref_object NMModemOfono *self = NM_MODEM_OFONO (user_data); - NMModemOfonoPrivate *priv = NM_MODEM_OFONO_GET_PRIVATE (self); - GError *error = NULL; + NMModemOfono *self; + NMModemOfonoPrivate *priv; + gs_free_error GError *error = NULL; + GDBusProxy *proxy; - priv->modem_proxy = g_dbus_proxy_new_for_bus_finish (result, &error); - if (error) { + proxy = g_dbus_proxy_new_for_bus_finish (result, &error); + if ( !proxy + && g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + return; + + self = NM_MODEM_OFONO (user_data); + priv = NM_MODEM_OFONO_GET_PRIVATE (self); + + if (!proxy) { _LOGE ("failed to create ofono modem DBus proxy: %s", error->message); + g_clear_object (&priv->modem_proxy_cancellable); return; } - /* Watch for custom ofono PropertyChanged signals */ + priv->modem_proxy = proxy; + _nm_dbus_signal_connect (priv->modem_proxy, "PropertyChanged", G_VARIANT_TYPE ("(sv)"), @@ -1178,9 +1199,9 @@ modem_proxy_new_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data) NULL, G_DBUS_CALL_FLAGS_NONE, 20000, - NULL, - (GAsyncReadyCallback) modem_get_properties_done, - g_object_ref (self)); + priv->modem_proxy_cancellable, + modem_get_properties_done, + self); } /*****************************************************************************/ @@ -1194,6 +1215,9 @@ static void constructed (GObject *object) { NMModemOfono *self = NM_MODEM_OFONO (object); + NMModemOfonoPrivate *priv = NM_MODEM_OFONO_GET_PRIVATE (self); + + priv->modem_proxy_cancellable = g_cancellable_new (); g_dbus_proxy_new_for_bus (G_BUS_TYPE_SYSTEM, G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START, @@ -1201,9 +1225,11 @@ constructed (GObject *object) OFONO_DBUS_SERVICE, nm_modem_get_path (NM_MODEM (self)), OFONO_DBUS_INTERFACE_MODEM, - NULL, - (GAsyncReadyCallback) modem_proxy_new_cb, - g_object_ref (self)); + priv->modem_proxy_cancellable, + modem_proxy_new_cb, + self); + + G_OBJECT_CLASS (nm_modem_ofono_parent_class)->constructed (object); } NMModem * @@ -1236,6 +1262,7 @@ dispose (GObject *object) NMModemOfono *self = NM_MODEM_OFONO (object); NMModemOfonoPrivate *priv = NM_MODEM_OFONO_GET_PRIVATE (self); + nm_clear_g_cancellable (&priv->modem_proxy_cancellable); nm_clear_g_cancellable (&priv->connman_proxy_cancellable); nm_clear_g_cancellable (&priv->sim_proxy_cancellable); @@ -1247,7 +1274,7 @@ dispose (GObject *object) g_clear_object (&priv->ip4_config); if (priv->modem_proxy) { - g_signal_handlers_disconnect_by_data (priv->modem_proxy, NM_MODEM_OFONO (self)); + g_signal_handlers_disconnect_by_data (priv->modem_proxy, self); g_clear_object (&priv->modem_proxy); } -- cgit v1.2.1 From 663dfd7d5132bad7def35573fcd683b6e181438d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 22 Apr 2017 22:54:23 +0200 Subject: ofono: make asynchrounous operations for context_proxy cancellable Also, - disconnect signal handlers in dispose() - fix memleak in stage1_prepare_down()() --- src/devices/wwan/nm-modem-ofono.c | 71 +++++++++++++++++++++++++++------------ 1 file changed, 50 insertions(+), 21 deletions(-) diff --git a/src/devices/wwan/nm-modem-ofono.c b/src/devices/wwan/nm-modem-ofono.c index 994f13f70e..2793fe887b 100644 --- a/src/devices/wwan/nm-modem-ofono.c +++ b/src/devices/wwan/nm-modem-ofono.c @@ -48,6 +48,7 @@ typedef struct { GCancellable *modem_proxy_cancellable; GCancellable *connman_proxy_cancellable; + GCancellable *context_proxy_cancellable; GCancellable *sim_proxy_cancellable; GError *property_error; @@ -796,15 +797,26 @@ modem_get_properties_done (GObject *source, } static void -stage1_prepare_done (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data) +stage1_prepare_done (GObject *source, + GAsyncResult *result, + gpointer user_data) { - gs_unref_object NMModemOfono *self = NM_MODEM_OFONO (user_data); - NMModemOfonoPrivate *priv = NM_MODEM_OFONO_GET_PRIVATE (self); - GError *error = NULL; + NMModemOfono *self; + NMModemOfonoPrivate *priv; + gs_free_error GError *error = NULL; + gs_unref_variant GVariant *v = NULL; + + v = g_dbus_proxy_call_finish (G_DBUS_PROXY (source), result, &error); + if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + return; + + self = NM_MODEM_OFONO (user_data); + priv = NM_MODEM_OFONO_GET_PRIVATE (self); + + g_clear_object (&priv->context_proxy_cancellable); g_clear_pointer (&priv->connect_properties, g_hash_table_destroy); - g_dbus_proxy_call_finish (proxy, result, &error); if (error) { _LOGW ("connection failed: %s", error->message); @@ -816,8 +828,6 @@ stage1_prepare_done (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data * leading to the connection being disabled, and a 5m * timeout... */ - - g_clear_error (&error); } } @@ -1030,21 +1040,33 @@ static_stage3_ip4_config_start (NMModem *modem, } static void -context_proxy_new_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data) +context_proxy_new_cb (GObject *source, GAsyncResult *result, gpointer user_data) { - gs_unref_object NMModemOfono *self = NM_MODEM_OFONO (user_data); - NMModemOfonoPrivate *priv = NM_MODEM_OFONO_GET_PRIVATE (self); - GError *error = NULL; + NMModemOfono *self; + NMModemOfonoPrivate *priv; + gs_free_error GError *error = NULL; + GDBusProxy *proxy; - priv->context_proxy = g_dbus_proxy_new_for_bus_finish (result, &error); - if (error) { + proxy = g_dbus_proxy_new_for_bus_finish (result, &error); + if ( !proxy + || g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + return; + + self = NM_MODEM_OFONO (user_data); + priv = NM_MODEM_OFONO_GET_PRIVATE (self); + + if (!proxy) { _LOGE ("failed to create ofono ConnectionContext DBus proxy: %s", error->message); + g_clear_object (&priv->context_proxy_cancellable); nm_modem_emit_prepare_result (NM_MODEM (self), FALSE, NM_DEVICE_STATE_REASON_MODEM_BUSY); return; } + priv->context_proxy = proxy; + if (!priv->gprs_attached) { + g_clear_object (&priv->context_proxy_cancellable); nm_modem_emit_prepare_result (NM_MODEM (self), FALSE, NM_DEVICE_STATE_REASON_MODEM_NO_CARRIER); return; @@ -1056,7 +1078,6 @@ context_proxy_new_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_dat */ g_clear_object (&priv->ip4_config); - /* Watch for custom ofono PropertyChanged signals */ _nm_dbus_signal_connect (priv->context_proxy, "PropertyChanged", G_VARIANT_TYPE ("(sv)"), @@ -1070,9 +1091,9 @@ context_proxy_new_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_dat g_variant_new ("b", TRUE)), G_DBUS_CALL_FLAGS_NONE, 20000, - NULL, - (GAsyncReadyCallback) stage1_prepare_done, - g_object_ref (self)); + priv->context_proxy_cancellable, + stage1_prepare_done, + self); } static void @@ -1082,16 +1103,20 @@ do_context_activate (NMModemOfono *self) g_return_if_fail (NM_IS_MODEM_OFONO (self)); + nm_clear_g_cancellable (&priv->context_proxy_cancellable); g_clear_object (&priv->context_proxy); + + priv->context_proxy_cancellable = g_cancellable_new (); + g_dbus_proxy_new_for_bus (G_BUS_TYPE_SYSTEM, G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START, NULL, OFONO_DBUS_SERVICE, priv->context_path, OFONO_DBUS_INTERFACE_CONNECTION_CONTEXT, - NULL, - (GAsyncReadyCallback) context_proxy_new_cb, - g_object_ref (self)); + priv->context_proxy_cancellable, + context_proxy_new_cb, + self); } static GHashTable * @@ -1264,6 +1289,7 @@ dispose (GObject *object) nm_clear_g_cancellable (&priv->modem_proxy_cancellable); nm_clear_g_cancellable (&priv->connman_proxy_cancellable); + nm_clear_g_cancellable (&priv->context_proxy_cancellable); nm_clear_g_cancellable (&priv->sim_proxy_cancellable); if (priv->connect_properties) { @@ -1283,7 +1309,10 @@ dispose (GObject *object) g_clear_object (&priv->connman_proxy); } - g_clear_object (&priv->context_proxy); + if (priv->context_proxy) { + g_signal_handlers_disconnect_by_data (priv->context_proxy, self); + g_clear_object (&priv->context_proxy); + } if (priv->sim_proxy) { g_signal_handlers_disconnect_by_data (priv->sim_proxy, self); -- cgit v1.2.1 From f3dfe0f745d2cfdab638804980e5e8501112183f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 22 Apr 2017 23:24:45 +0200 Subject: ofono: clenaup NMModemOfono's disconnect() - anticipate missing callback/ctx->result - always invoke the result callback when given - fix leaking GVariant in disconnect_done() - fix crash due to non-initialized ctx->result - pass cancellable to g_dbus_proxy_call() --- src/devices/wwan/nm-modem-ofono.c | 72 +++++++++++++++++++++------------------ 1 file changed, 38 insertions(+), 34 deletions(-) diff --git a/src/devices/wwan/nm-modem-ofono.c b/src/devices/wwan/nm-modem-ofono.c index 2793fe887b..d1ad016a91 100644 --- a/src/devices/wwan/nm-modem-ofono.c +++ b/src/devices/wwan/nm-modem-ofono.c @@ -170,28 +170,16 @@ typedef struct { static void disconnect_context_complete (DisconnectContext *ctx) { - g_simple_async_result_complete_in_idle (ctx->result); if (ctx->cancellable) g_object_unref (ctx->cancellable); - g_object_unref (ctx->result); + if (ctx->result) { + g_simple_async_result_complete_in_idle (ctx->result); + g_object_unref (ctx->result); + } g_object_unref (ctx->self); g_slice_free (DisconnectContext, ctx); } -static gboolean -disconnect_context_complete_if_cancelled (DisconnectContext *ctx) -{ - GError *error = NULL; - - if (g_cancellable_set_error_if_cancelled (ctx->cancellable, &error)) { - g_simple_async_result_take_error (ctx->result, error); - disconnect_context_complete (ctx); - return TRUE; - } - - return FALSE; -} - static gboolean disconnect_finish (NMModem *self, GAsyncResult *result, @@ -201,25 +189,25 @@ disconnect_finish (NMModem *self, } static void -disconnect_done (GDBusProxy *proxy, +disconnect_done (GObject *source, GAsyncResult *result, gpointer user_data) { DisconnectContext *ctx = (DisconnectContext*) user_data; NMModemOfono *self = ctx->self; - GError *error = NULL; + gs_free_error GError *error = NULL; + gs_unref_variant GVariant *v = NULL; - g_dbus_proxy_call_finish (proxy, result, &error); + v = g_dbus_proxy_call_finish (G_DBUS_PROXY (source), result, &error); if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) { - _LOGD ("disconnect cancelled"); + if (ctx->result) + g_simple_async_result_take_error (ctx->result, g_steal_pointer (&error)); + disconnect_context_complete (ctx); return; } - if (error) { - if (ctx->warn) - _LOGW ("failed to disconnect modem: %s", error->message); - g_clear_error (&error); - } + if (error && ctx->warn) + _LOGW ("failed to disconnect modem: %s", error->message); _LOGD ("modem disconnected"); @@ -238,18 +226,15 @@ disconnect (NMModem *modem, NMModemOfonoPrivate *priv = NM_MODEM_OFONO_GET_PRIVATE (self); DisconnectContext *ctx; NMModemState state = nm_modem_get_state (NM_MODEM (self)); + GError *error = NULL; _LOGD ("warn: %s modem_state: %s", warn ? "TRUE" : "FALSE", nm_modem_state_to_string (state)); - if (state != NM_MODEM_STATE_CONNECTED) - return; - - ctx = g_slice_new (DisconnectContext); + ctx = g_slice_new0 (DisconnectContext); ctx->self = g_object_ref (self); ctx->warn = warn; - if (callback) { ctx->result = g_simple_async_result_new (G_OBJECT (self), callback, @@ -257,9 +242,28 @@ disconnect (NMModem *modem, disconnect); } - ctx->cancellable = cancellable ? g_object_ref (cancellable) : NULL; - if (disconnect_context_complete_if_cancelled (ctx)) + if (state != NM_MODEM_STATE_CONNECTED) { + if (ctx->result) { + g_set_error_literal (&error, + NM_UTILS_ERROR, + NM_UTILS_ERROR_UNKNOWN, + ("modem is currently not connected")); + g_simple_async_result_take_error (ctx->result, error); + } + disconnect_context_complete (ctx); return; + } + + if (g_cancellable_set_error_if_cancelled (cancellable, &error)) { + if (ctx->result) + g_simple_async_result_take_error (ctx->result, error); + else + g_clear_error (&error); + disconnect_context_complete (ctx); + return; + } + + ctx->cancellable = nm_g_object_ref (cancellable); nm_modem_set_state (NM_MODEM (self), NM_MODEM_STATE_DISCONNECTING, @@ -272,8 +276,8 @@ disconnect (NMModem *modem, g_variant_new ("b", warn)), G_DBUS_CALL_FLAGS_NONE, 20000, - NULL, - (GAsyncReadyCallback) disconnect_done, + ctx->cancellable, + disconnect_done, ctx); } -- cgit v1.2.1 From 5bfb7c3c89afe4da5b1ac2395391e9a986c722f0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 23 Apr 2017 00:40:46 +0200 Subject: hostname: split out hostname management from NMSettings Hostname management is complicated. At least, how it is implemented currently. For example, NMPolicy also sets the hostname (NMPolicy calls nm_settings_set_transient_hostname() to have hostnamed set the hostname, but then falls back to sethostname() in settings_set_hostname_cb()). Also, NMManager tracks the hostname in NM_MANAGER_HOSTNAME too, and NMPolicy listens to changes from there -- instead of changes from NMSettings. Eventually, NMHostnameManager should contain the hostname parts from NMSettings and NMPolicy. --- Makefile.am | 3 + src/nm-hostname-manager.c | 649 +++++++++++++++++++++++++++++++++++++++++++++ src/nm-hostname-manager.h | 61 +++++ src/nm-manager.c | 5 +- src/nm-policy.c | 15 +- src/settings/nm-settings.c | 520 +++--------------------------------- src/settings/nm-settings.h | 10 - 7 files changed, 764 insertions(+), 499 deletions(-) create mode 100644 src/nm-hostname-manager.c create mode 100644 src/nm-hostname-manager.h diff --git a/Makefile.am b/Makefile.am index 96443196d4..32b6ba65f8 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1391,6 +1391,9 @@ src_libNetworkManager_la_SOURCES = \ src/ppp/nm-ppp-manager.h \ src/ppp/nm-ppp-status.h \ \ + src/nm-hostname-manager.c \ + src/nm-hostname-manager.h \ + \ src/settings/nm-agent-manager.c \ src/settings/nm-agent-manager.h \ src/settings/nm-inotify-helper.c \ diff --git a/src/nm-hostname-manager.c b/src/nm-hostname-manager.c new file mode 100644 index 0000000000..31132082b5 --- /dev/null +++ b/src/nm-hostname-manager.c @@ -0,0 +1,649 @@ +/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */ +/* NetworkManager + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + * + * (C) Copyright 2017 Red Hat, Inc. + */ + +#include "nm-default.h" + +#include "nm-hostname-manager.h" + +#include +#include +#include + +#if HAVE_SELINUX +#include +#endif + +#include "nm-common-macros.h" +#include "nm-dbus-interface.h" +#include "nm-connection.h" +#include "nm-utils.h" +#include "nm-core-internal.h" + +#include "NetworkManagerUtils.h" +#include "nm-dispatcher.h" + +/*****************************************************************************/ + +#define HOSTNAMED_SERVICE_NAME "org.freedesktop.hostname1" +#define HOSTNAMED_SERVICE_PATH "/org/freedesktop/hostname1" +#define HOSTNAMED_SERVICE_INTERFACE "org.freedesktop.hostname1" + +#define HOSTNAME_FILE_DEFAULT "/etc/hostname" +#define HOSTNAME_FILE_UCASE_HOSTNAME "/etc/HOSTNAME" +#define HOSTNAME_FILE_GENTOO "/etc/conf.d/hostname" + +#if (defined(HOSTNAME_PERSIST_SUSE) + defined(HOSTNAME_PERSIST_SLACKWARE) + defined(HOSTNAME_PERSIST_GENTOO)) > 1 +#error "Can only define one of HOSTNAME_PERSIST_*" +#endif + +#if defined(HOSTNAME_PERSIST_SUSE) +#define HOSTNAME_FILE HOSTNAME_FILE_UCASE_HOSTNAME +#elif defined(HOSTNAME_PERSIST_SLACKWARE) +#define HOSTNAME_FILE HOSTNAME_FILE_UCASE_HOSTNAME +#elif defined(HOSTNAME_PERSIST_GENTOO) +#define HOSTNAME_FILE HOSTNAME_FILE_GENTOO +#else +#define HOSTNAME_FILE HOSTNAME_FILE_DEFAULT +#endif + +/*****************************************************************************/ + +NM_GOBJECT_PROPERTIES_DEFINE (NMHostnameManager, + PROP_HOSTNAME, +); + +typedef struct { + char *value; + GFileMonitor *monitor; + GFileMonitor *dhcp_monitor; + gulong monitor_id; + gulong dhcp_monitor_id; + GDBusProxy *hostnamed_proxy; +} NMHostnameManagerPrivate; + +struct _NMHostnameManager { + GObject parent; + NMHostnameManagerPrivate _priv; +}; + +struct _NMHostnameManagerClass { + GObjectClass parent; +}; + +G_DEFINE_TYPE (NMHostnameManager, nm_hostname_manager, G_TYPE_OBJECT); + +#define NM_HOSTNAME_MANAGER_GET_PRIVATE(self) _NM_GET_PRIVATE (self, NMHostnameManager, NM_IS_HOSTNAME_MANAGER) + +NM_DEFINE_SINGLETON_GETTER (NMHostnameManager, nm_hostname_manager_get, NM_TYPE_HOSTNAME_MANAGER); + +/*****************************************************************************/ + +#define _NMLOG_DOMAIN LOGD_CORE +#define _NMLOG(level, ...) __NMLOG_DEFAULT (level, _NMLOG_DOMAIN, "hostname", __VA_ARGS__) + +/*****************************************************************************/ + +#if defined(HOSTNAME_PERSIST_GENTOO) +static gchar * +read_hostname_gentoo (const char *path) +{ + gs_free char *contents = NULL; + gs_strfreev char **all_lines = NULL; + const char *tmp; + guint i; + + if (!g_file_get_contents (path, &contents, NULL, NULL)) + return NULL; + + all_lines = g_strsplit (contents, "\n", 0); + for (i = 0; all_lines[i]; i++) { + g_strstrip (all_lines[i]); + if (all_lines[i][0] == '#' || all_lines[i][0] == '\0') + continue; + if (g_str_has_prefix (all_lines[i], "hostname=")) { + tmp = &all_lines[i][NM_STRLEN ("hostname=")]; + return g_shell_unquote (tmp, NULL); + } + } + return NULL; +} +#endif + +#if defined(HOSTNAME_PERSIST_SLACKWARE) +static gchar * +read_hostname_slackware (const char *path) +{ + gs_free char *contents = NULL; + gs_strfreev char **all_lines = NULL; + char *tmp; + guint i, j = 0; + + if (!g_file_get_contents (path, &contents, NULL, NULL)) + return NULL; + + all_lines = g_strsplit (contents, "\n", 0); + for (i = 0; all_lines[i]; i++) { + g_strstrip (all_lines[i]); + if (all_lines[i][0] == '#' || all_lines[i][0] == '\0') + continue; + tmp = &all_lines[i][0]; + /* We only want up to the first '.' -- the rest of the */ + /* fqdn is defined in /etc/hosts */ + while (tmp[j] != '\0') { + if (tmp[j] == '.') { + tmp[j] = '\0'; + break; + } + j++; + } + return g_shell_unquote (tmp, NULL); + } + return NULL; +} +#endif + +#if defined(HOSTNAME_PERSIST_SUSE) +static gboolean +hostname_is_dynamic (void) +{ + GIOChannel *channel; + char *str = NULL; + gboolean dynamic = FALSE; + + channel = g_io_channel_new_file (CONF_DHCP, "r", NULL); + if (!channel) + return dynamic; + + while (g_io_channel_read_line (channel, &str, NULL, NULL, NULL) != G_IO_STATUS_EOF) { + if (str) { + g_strstrip (str); + if (g_str_has_prefix (str, "DHCLIENT_SET_HOSTNAME=")) + dynamic = strcmp (&str[NM_STRLEN ("DHCLIENT_SET_HOSTNAME=")], "\"yes\"") == 0; + g_free (str); + } + } + + g_io_channel_shutdown (channel, FALSE, NULL); + g_io_channel_unref (channel); + + return dynamic; +} +#endif + +/* Returns an allocated string which the caller owns and must eventually free */ +char * +nm_hostname_manager_get_hostname (NMHostnameManager *self) +{ + NMHostnameManagerPrivate *priv = NM_HOSTNAME_MANAGER_GET_PRIVATE (self); + char *hostname = NULL; + + if (priv->hostnamed_proxy) { + hostname = g_strdup (priv->value); + goto out; + } + +#if defined(HOSTNAME_PERSIST_SUSE) + if (priv->dhcp_monitor_id && hostname_is_dynamic ()) + return NULL; +#endif + +#if defined(HOSTNAME_PERSIST_GENTOO) + hostname = read_hostname_gentoo (HOSTNAME_FILE); +#elif defined(HOSTNAME_PERSIST_SLACKWARE) + hostname = read_hostname_slackware (HOSTNAME_FILE); +#else + if (g_file_get_contents (HOSTNAME_FILE, &hostname, NULL, NULL)) + g_strchomp (hostname); +#endif + +out: + if (hostname && !hostname[0]) { + g_free (hostname); + hostname = NULL; + } + + return hostname; +} + +/*****************************************************************************/ + +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; + + 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, error->message); + } + + info->cb (info->hostname, !error, info->user_data); + g_free (info->hostname); +} + +void +nm_hostname_manager_set_transient_hostname (NMHostnameManager *self, + const char *hostname, + NMHostnameManagerSetHostnameCb cb, + gpointer user_data) +{ + NMHostnameManagerPrivate *priv; + SetHostnameInfo *info; + + g_return_if_fail (NM_IS_HOSTNAME_MANAGER (self)); + + priv = NM_HOSTNAME_MANAGER_GET_PRIVATE (self); + + if (!priv->hostnamed_proxy) { + cb (hostname, FALSE, user_data); + 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), + G_DBUS_CALL_FLAGS_NONE, + -1, + NULL, + set_transient_hostname_done, + info); +} + +gboolean +nm_hostname_manager_get_transient_hostname (NMHostnameManager *self, char **hostname) +{ + NMHostnameManagerPrivate *priv = NM_HOSTNAME_MANAGER_GET_PRIVATE (self); + GVariant *v_hostname; + + if (!priv->hostnamed_proxy) + return FALSE; + + v_hostname = g_dbus_proxy_get_cached_property (priv->hostnamed_proxy, + "Hostname"); + if (!v_hostname) { + _LOGT ("transient hostname retrieval failed"); + return FALSE; + } + + *hostname = g_variant_dup_string (v_hostname, NULL); + g_variant_unref (v_hostname); + + return TRUE; +} + +gboolean +nm_hostname_manager_write_hostname (NMHostnameManager *self, const char *hostname) +{ + NMHostnameManagerPrivate *priv; + char *hostname_eol; + gboolean ret; + gs_free_error GError *error = NULL; + const char *file = HOSTNAME_FILE; + gs_free char *link_path = NULL; + gs_unref_variant GVariant *var = NULL; + struct stat file_stat; +#if HAVE_SELINUX + security_context_t se_ctx_prev = NULL, se_ctx = NULL; + mode_t st_mode = 0; +#endif + + g_return_val_if_fail (NM_IS_HOSTNAME_MANAGER (self), FALSE); + + priv = NM_HOSTNAME_MANAGER_GET_PRIVATE (self); + + if (priv->hostnamed_proxy) { + var = g_dbus_proxy_call_sync (priv->hostnamed_proxy, + "SetStaticHostname", + g_variant_new ("(sb)", hostname, FALSE), + G_DBUS_CALL_FLAGS_NONE, + -1, + NULL, + &error); + if (error) + _LOGW ("could not set hostname: %s", error->message); + + return !error; + } + + /* If the hostname file is a symbolic link, follow it to find where the + * real file is located, otherwise g_file_set_contents will attempt to + * replace the link with a plain file. + */ + if ( lstat (file, &file_stat) == 0 + && S_ISLNK (file_stat.st_mode) + && (link_path = nm_utils_read_link_absolute (file, NULL))) + file = link_path; + +#if HAVE_SELINUX + /* Get default context for hostname file and set it for fscreate */ + if (stat (file, &file_stat) == 0) + st_mode = file_stat.st_mode; + matchpathcon (file, st_mode, &se_ctx); + matchpathcon_fini (); + getfscreatecon (&se_ctx_prev); + setfscreatecon (se_ctx); +#endif + +#if defined (HOSTNAME_PERSIST_GENTOO) + hostname_eol = g_strdup_printf ("#Generated by NetworkManager\n" + "hostname=\"%s\"\n", hostname); +#else + hostname_eol = g_strdup_printf ("%s\n", hostname); +#endif + + ret = g_file_set_contents (file, hostname_eol, -1, &error); + +#if HAVE_SELINUX + /* Restore previous context and cleanup */ + setfscreatecon (se_ctx_prev); + freecon (se_ctx); + freecon (se_ctx_prev); +#endif + + g_free (hostname_eol); + + if (!ret) { + _LOGW ("could not save hostname to %s: %s", file, error->message); + return FALSE; + } + + 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_maybe_changed (NMHostnameManager *settings) +{ + NMHostnameManagerPrivate *priv = NM_HOSTNAME_MANAGER_GET_PRIVATE (settings); + char *new_hostname; + + new_hostname = nm_hostname_manager_get_hostname (settings); + + if ( (new_hostname && !priv->value) + || (!new_hostname && priv->value) + || (priv->value && new_hostname && strcmp (priv->value, new_hostname))) { + + _LOGI ("hostname changed from %s%s%s to %s%s%s", + NM_PRINT_FMT_QUOTED (priv->value, "\"", priv->value, "\"", "(none)"), + NM_PRINT_FMT_QUOTED (new_hostname, "\"", new_hostname, "\"", "(none)")); + g_free (priv->value); + priv->value = new_hostname; + _notify (settings, PROP_HOSTNAME); + } else + g_free (new_hostname); +} + +static void +hostname_file_changed_cb (GFileMonitor *monitor, + GFile *file, + GFile *other_file, + GFileMonitorEvent event_type, + gpointer user_data) +{ + hostname_maybe_changed (user_data); +} + +/*****************************************************************************/ + +static void +hostnamed_properties_changed (GDBusProxy *proxy, + GVariant *changed_properties, + char **invalidated_properties, + gpointer user_data) +{ + NMHostnameManager *self = user_data; + NMHostnameManagerPrivate *priv = NM_HOSTNAME_MANAGER_GET_PRIVATE (self); + GVariant *v_hostname; + const char *hostname; + + v_hostname = g_dbus_proxy_get_cached_property (priv->hostnamed_proxy, + "StaticHostname"); + if (!v_hostname) + return; + + hostname = g_variant_get_string (v_hostname, NULL); + + if (g_strcmp0 (priv->value, hostname) != 0) { + _LOGI ("hostname changed from %s%s%s to %s%s%s", + NM_PRINT_FMT_QUOTED (priv->value, "\"", priv->value, "\"", "(none)"), + NM_PRINT_FMT_QUOTED (hostname, "\"", hostname, "\"", "(none)")); + g_free (priv->value); + priv->value = g_strdup (hostname); + _notify (self, PROP_HOSTNAME); + nm_dispatcher_call_hostname (NULL, NULL, NULL); + } + + g_variant_unref (v_hostname); +} + +static void +setup_hostname_file_monitors (NMHostnameManager *self) +{ + NMHostnameManagerPrivate *priv = NM_HOSTNAME_MANAGER_GET_PRIVATE (self); + GFileMonitor *monitor; + const char *path = HOSTNAME_FILE; + char *link_path = NULL; + struct stat file_stat; + GFile *file; + + priv->value = nm_hostname_manager_get_hostname (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) + && (link_path = nm_utils_read_link_absolute (path, NULL))) { + path = link_path; + if ( lstat(link_path, &file_stat) == 0 + && S_ISLNK (file_stat.st_mode)) { + _LOGW ("only one level of symbolic link indirection is allowed when monitoring " + HOSTNAME_FILE); + } + } + + /* 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); + if (monitor) { + priv->monitor_id = g_signal_connect (monitor, "changed", + G_CALLBACK (hostname_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); + if (monitor) { + priv->dhcp_monitor_id = g_signal_connect (monitor, "changed", + G_CALLBACK (hostname_file_changed_cb), + self); + priv->dhcp_monitor = monitor; + } +#endif + + hostname_maybe_changed (self); +} + +/*****************************************************************************/ + +static void +get_property (GObject *object, guint prop_id, + GValue *value, GParamSpec *pspec) +{ + NMHostnameManager *self = NM_HOSTNAME_MANAGER (object); + + switch (prop_id) { + case PROP_HOSTNAME: + g_value_take_string (value, nm_hostname_manager_get_hostname (self)); + + /* Don't ever pass NULL through D-Bus */ + if (!g_value_get_string (value)) + g_value_set_static_string (value, ""); + break; + default: + G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); + break; + } +} + +/*****************************************************************************/ + +static void +nm_hostname_manager_init (NMHostnameManager *self) +{ +} + +static void +constructed (GObject *object) +{ + NMHostnameManager *self = NM_HOSTNAME_MANAGER (object); + NMHostnameManagerPrivate *priv = NM_HOSTNAME_MANAGER_GET_PRIVATE (self); + GDBusProxy *proxy; + GVariant *variant; + gs_free GError *error = NULL; + + proxy = g_dbus_proxy_new_for_bus_sync (G_BUS_TYPE_SYSTEM, 0, NULL, + HOSTNAMED_SERVICE_NAME, HOSTNAMED_SERVICE_PATH, + HOSTNAMED_SERVICE_INTERFACE, NULL, &error); + if (proxy) { + variant = g_dbus_proxy_get_cached_property (proxy, "StaticHostname"); + if (variant) { + _LOGI ("hostname: using hostnamed"); + priv->hostnamed_proxy = proxy; + g_signal_connect (proxy, "g-properties-changed", + G_CALLBACK (hostnamed_properties_changed), self); + hostnamed_properties_changed (proxy, NULL, NULL, self); + g_variant_unref (variant); + } else { + _LOGI ("hostname: couldn't get property from hostnamed"); + g_object_unref (proxy); + } + } else { + _LOGI ("hostname: hostnamed not used as proxy creation failed with: %s", + error->message); + g_clear_error (&error); + } + + if (!priv->hostnamed_proxy) + setup_hostname_file_monitors (self); + + G_OBJECT_CLASS (nm_hostname_manager_parent_class)->constructed (object); +} + +static void +dispose (GObject *object) +{ + NMHostnameManager *self = NM_HOSTNAME_MANAGER (object); + NMHostnameManagerPrivate *priv = NM_HOSTNAME_MANAGER_GET_PRIVATE (self); + + if (priv->hostnamed_proxy) { + g_signal_handlers_disconnect_by_func (priv->hostnamed_proxy, + G_CALLBACK (hostnamed_properties_changed), + self); + 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); + } + + g_clear_pointer (&priv->value, g_free); + + G_OBJECT_CLASS (nm_hostname_manager_parent_class)->dispose (object); +} + +static void +nm_hostname_manager_class_init (NMHostnameManagerClass *class) +{ + GObjectClass *object_class = G_OBJECT_CLASS (class); + + object_class->constructed = constructed; + object_class->get_property = get_property; + object_class->dispose = dispose; + + obj_properties[PROP_HOSTNAME] = + g_param_spec_string (NM_HOSTNAME_MANAGER_HOSTNAME, "", "", + NULL, + G_PARAM_READABLE | + G_PARAM_STATIC_STRINGS); + + g_object_class_install_properties (object_class, _PROPERTY_ENUMS_LAST, obj_properties); +} diff --git a/src/nm-hostname-manager.h b/src/nm-hostname-manager.h new file mode 100644 index 0000000000..92702bbc1b --- /dev/null +++ b/src/nm-hostname-manager.h @@ -0,0 +1,61 @@ +/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */ +/* NetworkManager + * + * Søren Sandmann + * Dan Williams + * Tambet Ingo + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + * + * (C) Copyright 2007 - 2011, 2017 Red Hat, Inc. + * (C) Copyright 2008 Novell, Inc. + */ + +#ifndef __NM_HOSTNAME_MANAGER_H__ +#define __NM_HOSTNAME_MANAGER_H__ + +#define NM_TYPE_HOSTNAME_MANAGER (nm_hostname_manager_get_type ()) +#define NM_HOSTNAME_MANAGER(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), NM_TYPE_HOSTNAME_MANAGER, NMHostnameManager)) +#define NM_HOSTNAME_MANAGER_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), NM_TYPE_HOSTNAME_MANAGER, NMHostnameManagerClass)) +#define NM_IS_HOSTNAME_MANAGER(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), NM_TYPE_HOSTNAME_MANAGER)) +#define NM_IS_HOSTNAME_MANAGER_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), NM_TYPE_HOSTNAME_MANAGER)) +#define NM_HOSTNAME_MANAGER_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), NM_TYPE_HOSTNAME_MANAGER, NMHostnameManagerClass)) + +#define NM_HOSTNAME_MANAGER_HOSTNAME "hostname" + +typedef struct _NMHostnameManager NMHostnameManager; +typedef struct _NMHostnameManagerClass NMHostnameManagerClass; + +typedef void (*NMHostnameManagerSetHostnameCb) (const char *name, gboolean result, gpointer user_data); + +GType nm_hostname_manager_get_type (void); + +NMHostnameManager *nm_hostname_manager_get (void); + +char *nm_hostname_manager_get_hostname (NMHostnameManager *self); + +gboolean nm_hostname_manager_write_hostname (NMHostnameManager *self, const char *hostname); + +void nm_hostname_manager_set_transient_hostname (NMHostnameManager *self, + const char *hostname, + NMHostnameManagerSetHostnameCb cb, + gpointer user_data); + +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/nm-manager.c b/src/nm-manager.c index a74021951a..0d65e58537 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -1412,7 +1412,10 @@ system_hostname_changed_cb (NMSettings *settings, NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); char *hostname; - hostname = nm_settings_get_hostname (priv->settings); + g_object_get (priv->settings, + NM_SETTINGS_HOSTNAME, + &hostname, + NULL); /* nm_settings_get_hostname() does not return an empty hostname. */ nm_assert (!hostname || *hostname); diff --git a/src/nm-policy.c b/src/nm-policy.c index 7c74a6b412..24b3cba8c4 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -49,6 +49,7 @@ #include "nm-dhcp6-config.h" #include "nm-config.h" #include "nm-netns.h" +#include "nm-hostname-manager.h" /*****************************************************************************/ @@ -73,6 +74,8 @@ typedef struct { NMSettings *settings; + NMHostnameManager *hostname_manager; + NMDevice *default_device4, *activating_device4; NMDevice *default_device6, *activating_device6; @@ -460,7 +463,7 @@ _get_hostname (NMPolicy *self, char **hostname) } /* try to get the hostname via dbus... */ - if (nm_settings_get_transient_hostname (priv->settings, hostname)) { + if (nm_hostname_manager_get_transient_hostname (priv->hostname_manager, hostname)) { _LOGT (LOGD_DNS, "get-hostname: \"%s\" (from dbus)", *hostname); return *hostname; } @@ -549,10 +552,10 @@ _set_hostname (NMPolicy *self, /* Ask NMSettings to update the transient hostname using its * systemd-hostnamed proxy */ - nm_settings_set_transient_hostname (priv->settings, - name, - settings_set_hostname_cb, - g_object_ref (self)); + nm_hostname_manager_set_transient_hostname (priv->hostname_manager, + name, + settings_set_hostname_cb, + g_object_ref (self)); } static void @@ -2300,6 +2303,8 @@ nm_policy_init (NMPolicy *self) priv->netns = g_object_ref (nm_netns_get ()); + priv->hostname_manager = g_object_ref (nm_hostname_manager_get ()); + hostname_mode = nm_config_data_get_value (NM_CONFIG_GET_DATA_ORIG, NM_CONFIG_KEYFILE_GROUP_MAIN, NM_CONFIG_KEYFILE_KEY_MAIN_HOSTNAME_MODE, diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index afd1b0849c..2e41f9c3a4 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -76,6 +76,7 @@ #include "NetworkManagerUtils.h" #include "nm-dispatcher.h" #include "nm-inotify-helper.h" +#include "nm-hostname-manager.h" #include "introspection/org.freedesktop.NetworkManager.Settings.h" @@ -94,13 +95,6 @@ EXPORT(nm_settings_connection_replace_and_commit) /*****************************************************************************/ -#define HOSTNAMED_SERVICE_NAME "org.freedesktop.hostname1" -#define HOSTNAMED_SERVICE_PATH "/org/freedesktop/hostname1" -#define HOSTNAMED_SERVICE_INTERFACE "org.freedesktop.hostname1" - -#define HOSTNAME_FILE_DEFAULT "/etc/hostname" -#define HOSTNAME_FILE_UCASE_HOSTNAME "/etc/HOSTNAME" -#define HOSTNAME_FILE_GENTOO "/etc/conf.d/hostname" #define IFCFG_DIR SYSCONFDIR "/sysconfig/network" #define CONF_DHCP IFCFG_DIR "/dhcp" @@ -162,14 +156,9 @@ typedef struct { gboolean started; gboolean startup_complete; - struct { - char *value; - GFileMonitor *monitor; - GFileMonitor *dhcp_monitor; - gulong monitor_id; - gulong dhcp_monitor_id; - GDBusProxy *hostnamed_proxy; - } hostname; + char *hostname; + NMHostnameManager *hostname_manager; + } NMSettingsPrivate; struct _NMSettings { @@ -568,131 +557,6 @@ get_plugin (NMSettings *self, guint32 capability) return NULL; } -#if defined(HOSTNAME_PERSIST_GENTOO) -static gchar * -read_hostname_gentoo (const char *path) -{ - gs_free char *contents = NULL; - gs_strfreev char **all_lines = NULL; - const char *tmp; - guint i; - - if (!g_file_get_contents (path, &contents, NULL, NULL)) - return NULL; - - all_lines = g_strsplit (contents, "\n", 0); - for (i = 0; all_lines[i]; i++) { - g_strstrip (all_lines[i]); - if (all_lines[i][0] == '#' || all_lines[i][0] == '\0') - continue; - if (g_str_has_prefix (all_lines[i], "hostname=")) { - tmp = &all_lines[i][NM_STRLEN ("hostname=")]; - return g_shell_unquote (tmp, NULL); - } - } - return NULL; -} -#endif - -#if defined(HOSTNAME_PERSIST_SLACKWARE) -static gchar * -read_hostname_slackware (const char *path) -{ - gs_free char *contents = NULL; - gs_strfreev char **all_lines = NULL; - char *tmp; - guint i, j = 0; - - if (!g_file_get_contents (path, &contents, NULL, NULL)) - return NULL; - - all_lines = g_strsplit (contents, "\n", 0); - for (i = 0; all_lines[i]; i++) { - g_strstrip (all_lines[i]); - if (all_lines[i][0] == '#' || all_lines[i][0] == '\0') - continue; - tmp = &all_lines[i][0]; - /* We only want up to the first '.' -- the rest of the */ - /* fqdn is defined in /etc/hosts */ - while (tmp[j] != '\0') { - if (tmp[j] == '.') { - tmp[j] = '\0'; - break; - } - j++; - } - return g_shell_unquote (tmp, NULL); - } - return NULL; -} -#endif - -#if defined(HOSTNAME_PERSIST_SUSE) -static gboolean -hostname_is_dynamic (void) -{ - GIOChannel *channel; - char *str = NULL; - gboolean dynamic = FALSE; - - channel = g_io_channel_new_file (CONF_DHCP, "r", NULL); - if (!channel) - return dynamic; - - while (g_io_channel_read_line (channel, &str, NULL, NULL, NULL) != G_IO_STATUS_EOF) { - if (str) { - g_strstrip (str); - if (g_str_has_prefix (str, "DHCLIENT_SET_HOSTNAME=")) - dynamic = strcmp (&str[NM_STRLEN ("DHCLIENT_SET_HOSTNAME=")], "\"yes\"") == 0; - g_free (str); - } - } - - g_io_channel_shutdown (channel, FALSE, NULL); - g_io_channel_unref (channel); - - return dynamic; -} -#endif - -/* Returns an allocated string which the caller owns and must eventually free */ -char * -nm_settings_get_hostname (NMSettings *self) -{ - NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self); - char *hostname = NULL; - - if (!priv->started) - return NULL; - - if (priv->hostname.hostnamed_proxy) { - hostname = g_strdup (priv->hostname.value); - goto out; - } - -#if defined(HOSTNAME_PERSIST_SUSE) - if (priv->hostname.dhcp_monitor_id && hostname_is_dynamic ()) - return NULL; -#endif - -#if defined(HOSTNAME_PERSIST_GENTOO) - hostname = read_hostname_gentoo (HOSTNAME_FILE); -#elif defined(HOSTNAME_PERSIST_SLACKWARE) - hostname = read_hostname_slackware (HOSTNAME_FILE); -#else - if (g_file_get_contents (HOSTNAME_FILE, &hostname, NULL, NULL)) - g_strchomp (hostname); -#endif - -out: - if (hostname && !hostname[0]) { - g_free (hostname); - hostname = NULL; - } - - return hostname; -} - static gboolean find_spec (GSList *spec_list, const char *spec) { @@ -1626,160 +1490,7 @@ impl_settings_reload_connections (NMSettings *self, g_dbus_method_invocation_return_value (context, g_variant_new ("(b)", TRUE)); } -typedef struct { - char *hostname; - NMSettingsSetHostnameCb 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; - - 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, error->message); - } - - info->cb (info->hostname, !error, info->user_data); - g_free (info->hostname); -} - -void -nm_settings_set_transient_hostname (NMSettings *self, - const char *hostname, - NMSettingsSetHostnameCb cb, - gpointer user_data) -{ - NMSettingsPrivate *priv; - SetHostnameInfo *info; - - g_return_if_fail (NM_IS_SETTINGS (self)); - priv = NM_SETTINGS_GET_PRIVATE (self); - - if (!priv->hostname.hostnamed_proxy) { - cb (hostname, FALSE, user_data); - return; - } - - info = g_new0 (SetHostnameInfo, 1); - info->hostname = g_strdup (hostname); - info->cb = cb; - info->user_data = user_data; - - g_dbus_proxy_call (priv->hostname.hostnamed_proxy, - "SetHostname", - g_variant_new ("(sb)", hostname, FALSE), - G_DBUS_CALL_FLAGS_NONE, - -1, - NULL, - set_transient_hostname_done, - info); -} - -gboolean -nm_settings_get_transient_hostname (NMSettings *self, char **hostname) -{ - NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self); - GVariant *v_hostname; - - if (!priv->hostname.hostnamed_proxy) - return FALSE; - - v_hostname = g_dbus_proxy_get_cached_property (priv->hostname.hostnamed_proxy, - "Hostname"); - if (!v_hostname) { - _LOGT ("transient hostname retrieval failed"); - return FALSE; - } - - *hostname = g_variant_dup_string (v_hostname, NULL); - g_variant_unref (v_hostname); - - return TRUE; -} - -static gboolean -write_hostname (NMSettingsPrivate *priv, const char *hostname) -{ - char *hostname_eol; - gboolean ret; - gs_free_error GError *error = NULL; - const char *file = HOSTNAME_FILE; - gs_free char *link_path = NULL; - gs_unref_variant GVariant *var = NULL; - struct stat file_stat; -#if HAVE_SELINUX - security_context_t se_ctx_prev = NULL, se_ctx = NULL; - mode_t st_mode = 0; -#endif - - if (priv->hostname.hostnamed_proxy) { - var = g_dbus_proxy_call_sync (priv->hostname.hostnamed_proxy, - "SetStaticHostname", - g_variant_new ("(sb)", hostname, FALSE), - G_DBUS_CALL_FLAGS_NONE, - -1, - NULL, - &error); - if (error) - _LOGW ("could not set hostname: %s", error->message); - - return !error; - } - - /* If the hostname file is a symbolic link, follow it to find where the - * real file is located, otherwise g_file_set_contents will attempt to - * replace the link with a plain file. - */ - if ( lstat (file, &file_stat) == 0 - && S_ISLNK (file_stat.st_mode) - && (link_path = nm_utils_read_link_absolute (file, NULL))) - file = link_path; - -#if HAVE_SELINUX - /* Get default context for hostname file and set it for fscreate */ - if (stat (file, &file_stat) == 0) - st_mode = file_stat.st_mode; - matchpathcon (file, st_mode, &se_ctx); - matchpathcon_fini (); - getfscreatecon (&se_ctx_prev); - setfscreatecon (se_ctx); -#endif - -#if defined (HOSTNAME_PERSIST_GENTOO) - hostname_eol = g_strdup_printf ("#Generated by NetworkManager\n" - "hostname=\"%s\"\n", hostname); -#else - hostname_eol = g_strdup_printf ("%s\n", hostname); -#endif - - ret = g_file_set_contents (file, hostname_eol, -1, &error); - -#if HAVE_SELINUX - /* Restore previous context and cleanup */ - setfscreatecon (se_ctx_prev); - freecon (se_ctx); - freecon (se_ctx_prev); -#endif - - g_free (hostname_eol); - - if (!ret) { - _LOGW ("could not save hostname to %s: %s", file, error->message); - return FALSE; - } - - return TRUE; -} +/*****************************************************************************/ static void pk_hostname_cb (NMAuthChain *chain, @@ -1812,7 +1523,7 @@ pk_hostname_cb (NMAuthChain *chain, } else { hostname = nm_auth_chain_get_data (chain, "hostname"); - if (!write_hostname (priv, hostname)) { + if (!nm_hostname_manager_write_hostname (priv->hostname_manager, hostname)) { error = g_error_new_literal (NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED, "Saving the hostname failed."); @@ -1827,33 +1538,6 @@ pk_hostname_cb (NMAuthChain *chain, nm_auth_chain_unref (chain); } -static gboolean -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 impl_settings_save_hostname (NMSettings *self, GDBusMethodInvocation *context, @@ -1864,7 +1548,7 @@ impl_settings_save_hostname (NMSettings *self, GError *error = NULL; /* Minimal validation of the hostname */ - if (!validate_hostname (hostname)) { + if (!nm_hostname_manager_validate_hostname (hostname)) { error = g_error_new_literal (NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_HOSTNAME, "The hostname was too long or contained invalid characters."); @@ -1888,37 +1572,7 @@ done: g_dbus_method_invocation_take_error (context, error); } -static void -hostname_maybe_changed (NMSettings *settings) -{ - NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (settings); - char *new_hostname; - - new_hostname = nm_settings_get_hostname (settings); - - if ( (new_hostname && !priv->hostname.value) - || (!new_hostname && priv->hostname.value) - || (priv->hostname.value && new_hostname && strcmp (priv->hostname.value, new_hostname))) { - - _LOGI ("hostname changed from %s%s%s to %s%s%s", - NM_PRINT_FMT_QUOTED (priv->hostname.value, "\"", priv->hostname.value, "\"", "(none)"), - NM_PRINT_FMT_QUOTED (new_hostname, "\"", new_hostname, "\"", "(none)")); - g_free (priv->hostname.value); - priv->hostname.value = new_hostname; - _notify (settings, PROP_HOSTNAME); - } else - g_free (new_hostname); -} - -static void -hostname_file_changed_cb (GFileMonitor *monitor, - GFile *file, - GFile *other_file, - GFileMonitorEvent event_type, - gpointer user_data) -{ - hostname_maybe_changed (user_data); -} +/*****************************************************************************/ static gboolean have_connection_for_device (NMSettings *self, NMDevice *device) @@ -2141,95 +1795,34 @@ nm_settings_get_startup_complete (NMSettings *self) /*****************************************************************************/ static void -hostnamed_properties_changed (GDBusProxy *proxy, - GVariant *changed_properties, - char **invalidated_properties, - gpointer user_data) +_hostname_changed (NMSettings *self) { - NMSettings *self = user_data; NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self); - GVariant *v_hostname; - const char *hostname; - - v_hostname = g_dbus_proxy_get_cached_property (priv->hostname.hostnamed_proxy, - "StaticHostname"); - if (!v_hostname) - return; + gs_free char *hostname = NULL; - hostname = g_variant_get_string (v_hostname, NULL); + hostname = nm_hostname_manager_get_hostname (priv->hostname_manager); - if (g_strcmp0 (priv->hostname.value, hostname) != 0) { - _LOGI ("hostname changed from %s%s%s to %s%s%s", - NM_PRINT_FMT_QUOTED (priv->hostname.value, "\"", priv->hostname.value, "\"", "(none)"), - NM_PRINT_FMT_QUOTED (hostname, "\"", hostname, "\"", "(none)")); - g_free (priv->hostname.value); - priv->hostname.value = g_strdup (hostname); - _notify (self, PROP_HOSTNAME); - nm_dispatcher_call_hostname (NULL, NULL, NULL); - } - - g_variant_unref (v_hostname); + if (nm_streq0 (hostname, priv->hostname)) + return; + g_free (priv->hostname); + priv->hostname = g_steal_pointer (&hostname); + _notify (self, PROP_HOSTNAME); } static void -setup_hostname_file_monitors (NMSettings *self) +_hostname_changed_cb (NMHostnameManager *hostname_manager, + GParamSpec *pspec, + gpointer user_data) { - NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self); - GFileMonitor *monitor; - const char *path = HOSTNAME_FILE; - char *link_path = NULL; - struct stat file_stat; - GFile *file; - - priv->hostname.value = nm_settings_get_hostname (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) - && (link_path = nm_utils_read_link_absolute (path, NULL))) { - path = link_path; - if ( lstat(link_path, &file_stat) == 0 - && S_ISLNK (file_stat.st_mode)) { - _LOGW ("only one level of symbolic link indirection is allowed when monitoring " - HOSTNAME_FILE); - } - } - - /* 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); - if (monitor) { - priv->hostname.monitor_id = g_signal_connect (monitor, "changed", - G_CALLBACK (hostname_file_changed_cb), - self); - priv->hostname.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); - if (monitor) { - priv->hostname.dhcp_monitor_id = g_signal_connect (monitor, "changed", - G_CALLBACK (hostname_file_changed_cb), - self); - priv->hostname.dhcp_monitor = monitor; - } -#endif - - hostname_maybe_changed (self); + _hostname_changed (NM_SETTINGS (user_data)); } +/*****************************************************************************/ + gboolean nm_settings_start (NMSettings *self, GError **error) { NMSettingsPrivate *priv; - GDBusProxy *proxy; - GVariant *variant; - GError *local_error = NULL; gs_strfreev char **plugins = NULL; priv = NM_SETTINGS_GET_PRIVATE (self); @@ -2245,33 +1838,12 @@ nm_settings_start (NMSettings *self, GError **error) load_connections (self); check_startup_complete (self); - proxy = g_dbus_proxy_new_for_bus_sync (G_BUS_TYPE_SYSTEM, 0, NULL, - HOSTNAMED_SERVICE_NAME, HOSTNAMED_SERVICE_PATH, - HOSTNAMED_SERVICE_INTERFACE, NULL, &local_error); - if (proxy) { - variant = g_dbus_proxy_get_cached_property (proxy, "StaticHostname"); - if (variant) { - _LOGI ("hostname: using hostnamed"); - priv->hostname.hostnamed_proxy = proxy; - g_signal_connect (proxy, "g-properties-changed", - G_CALLBACK (hostnamed_properties_changed), self); - hostnamed_properties_changed (proxy, NULL, NULL, self); - g_variant_unref (variant); - } else { - _LOGI ("hostname: couldn't get property from hostnamed"); - g_object_unref (proxy); - } - } else { - _LOGI ("hostname: hostnamed not used as proxy creation failed with: %s", - local_error->message); - g_clear_error (&local_error); - } - - if (!priv->hostname.hostnamed_proxy) - setup_hostname_file_monitors (self); - - priv->started = TRUE; - _notify (self, PROP_HOSTNAME); + priv->hostname_manager = g_object_ref (nm_hostname_manager_get ()); + g_signal_connect (priv->hostname_manager, + "notify::"NM_HOSTNAME_MANAGER_HOSTNAME, + G_CALLBACK (_hostname_changed_cb), + self); + _hostname_changed (self); return TRUE; } @@ -2298,10 +1870,9 @@ get_property (GObject *object, guint prop_id, g_value_take_boxed (value, (char **) g_ptr_array_free (array, FALSE)); break; case PROP_HOSTNAME: - g_value_take_string (value, nm_settings_get_hostname (self)); - - /* Don't ever pass NULL through D-Bus */ - if (!g_value_get_string (value)) + if (priv->hostname) + g_value_set_string (value, priv->hostname); + else g_value_set_static_string (value, ""); break; case PROP_CAN_MODIFY: @@ -2362,32 +1933,13 @@ dispose (GObject *object) g_object_unref (priv->agent_mgr); - if (priv->hostname.hostnamed_proxy) { - g_signal_handlers_disconnect_by_func (priv->hostname.hostnamed_proxy, - G_CALLBACK (hostnamed_properties_changed), + if (priv->hostname_manager) { + g_signal_handlers_disconnect_by_func (priv->hostname_manager, + G_CALLBACK (_hostname_changed_cb), self); - g_clear_object (&priv->hostname.hostnamed_proxy); - } - - if (priv->hostname.monitor) { - if (priv->hostname.monitor_id) - g_signal_handler_disconnect (priv->hostname.monitor, priv->hostname.monitor_id); - - g_file_monitor_cancel (priv->hostname.monitor); - g_clear_object (&priv->hostname.monitor); + g_clear_object (&priv->hostname_manager); } - if (priv->hostname.dhcp_monitor) { - if (priv->hostname.dhcp_monitor_id) - g_signal_handler_disconnect (priv->hostname.dhcp_monitor, - priv->hostname.dhcp_monitor_id); - - g_file_monitor_cancel (priv->hostname.dhcp_monitor); - g_clear_object (&priv->hostname.dhcp_monitor); - } - - g_clear_pointer (&priv->hostname.value, g_free); - G_OBJECT_CLASS (nm_settings_parent_class)->dispose (object); } @@ -2405,6 +1957,8 @@ finalize (GObject *object) g_slist_free_full (priv->plugins, g_object_unref); + g_free (priv->hostname); + g_clear_object (&priv->config); G_OBJECT_CLASS (nm_settings_parent_class)->finalize (object); diff --git a/src/settings/nm-settings.h b/src/settings/nm-settings.h index 7110a12ba8..eede76b029 100644 --- a/src/settings/nm-settings.h +++ b/src/settings/nm-settings.h @@ -119,20 +119,10 @@ gboolean nm_settings_has_connection (NMSettings *self, NMSettingsConnection *con const GSList *nm_settings_get_unmanaged_specs (NMSettings *self); -char *nm_settings_get_hostname (NMSettings *self); - void nm_settings_device_added (NMSettings *self, NMDevice *device); void nm_settings_device_removed (NMSettings *self, NMDevice *device, gboolean quitting); gboolean nm_settings_get_startup_complete (NMSettings *self); -void nm_settings_set_transient_hostname (NMSettings *self, - const char *hostname, - NMSettingsSetHostnameCb cb, - gpointer user_data); - -gboolean nm_settings_get_transient_hostname (NMSettings *self, - char **hostname); - #endif /* __NM_SETTINGS_H__ */ -- cgit v1.2.1 From 54f5407abf938af436f3cf65687bc9004259416d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 23 Apr 2017 14:20:37 +0200 Subject: hostname: cache hostname-manager's hostname property A property preferably only emits a notify-changed signal when the value actually changes and it caches the value (so that between property-changed signals the value is guaranteed not to change). NMSettings and NMManager both already cache the hostname, because NMHostnameManager didn't guarantee this basic concept. Implement it and rely on it from NMSettings and NMPolicy. And remove the copy of the property from NMManager. Move the call for nm_dispatcher_call_hostname() from NMHostnameManager to NMManager. Note that NMPolicy also has a call to the dispatcher when set-transient-hostname returns. This should be cleaned up later. --- src/nm-hostname-manager.c | 118 +++++++++++++++++++++++++-------------------- src/nm-hostname-manager.h | 4 +- src/nm-manager.c | 67 ++++++++----------------- src/nm-manager.h | 1 - src/nm-policy.c | 16 +++--- src/settings/nm-settings.c | 32 +++--------- 6 files changed, 107 insertions(+), 131 deletions(-) diff --git a/src/nm-hostname-manager.c b/src/nm-hostname-manager.c index 31132082b5..40f3351a69 100644 --- a/src/nm-hostname-manager.c +++ b/src/nm-hostname-manager.c @@ -37,7 +37,6 @@ #include "nm-core-internal.h" #include "NetworkManagerUtils.h" -#include "nm-dispatcher.h" /*****************************************************************************/ @@ -70,7 +69,7 @@ NM_GOBJECT_PROPERTIES_DEFINE (NMHostnameManager, ); typedef struct { - char *value; + char *current_hostname; GFileMonitor *monitor; GFileMonitor *dhcp_monitor; gulong monitor_id; @@ -189,13 +188,13 @@ hostname_is_dynamic (void) /* Returns an allocated string which the caller owns and must eventually free */ char * -nm_hostname_manager_get_hostname (NMHostnameManager *self) +nm_hostname_manager_read_hostname (NMHostnameManager *self) { NMHostnameManagerPrivate *priv = NM_HOSTNAME_MANAGER_GET_PRIVATE (self); char *hostname = NULL; if (priv->hostnamed_proxy) { - hostname = g_strdup (priv->value); + hostname = g_strdup (priv->current_hostname); goto out; } @@ -216,7 +215,7 @@ nm_hostname_manager_get_hostname (NMHostnameManager *self) out: if (hostname && !hostname[0]) { g_free (hostname); - hostname = NULL; + return NULL; } return hostname; @@ -224,6 +223,60 @@ out: /*****************************************************************************/ +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); +} + +/*****************************************************************************/ + typedef struct { char *hostname; NMHostnameManagerSetHostnameCb cb; @@ -412,28 +465,6 @@ nm_hostname_manager_validate_hostname (const char *hostname) return (p - hostname <= HOST_NAME_MAX); } -static void -hostname_maybe_changed (NMHostnameManager *settings) -{ - NMHostnameManagerPrivate *priv = NM_HOSTNAME_MANAGER_GET_PRIVATE (settings); - char *new_hostname; - - new_hostname = nm_hostname_manager_get_hostname (settings); - - if ( (new_hostname && !priv->value) - || (!new_hostname && priv->value) - || (priv->value && new_hostname && strcmp (priv->value, new_hostname))) { - - _LOGI ("hostname changed from %s%s%s to %s%s%s", - NM_PRINT_FMT_QUOTED (priv->value, "\"", priv->value, "\"", "(none)"), - NM_PRINT_FMT_QUOTED (new_hostname, "\"", new_hostname, "\"", "(none)")); - g_free (priv->value); - priv->value = new_hostname; - _notify (settings, PROP_HOSTNAME); - } else - g_free (new_hostname); -} - static void hostname_file_changed_cb (GFileMonitor *monitor, GFile *file, @@ -441,7 +472,7 @@ hostname_file_changed_cb (GFileMonitor *monitor, GFileMonitorEvent event_type, gpointer user_data) { - hostname_maybe_changed (user_data); + _set_hostname_read (user_data); } /*****************************************************************************/ @@ -455,26 +486,13 @@ hostnamed_properties_changed (GDBusProxy *proxy, NMHostnameManager *self = user_data; NMHostnameManagerPrivate *priv = NM_HOSTNAME_MANAGER_GET_PRIVATE (self); GVariant *v_hostname; - const char *hostname; v_hostname = g_dbus_proxy_get_cached_property (priv->hostnamed_proxy, "StaticHostname"); - if (!v_hostname) - return; - - hostname = g_variant_get_string (v_hostname, NULL); - - if (g_strcmp0 (priv->value, hostname) != 0) { - _LOGI ("hostname changed from %s%s%s to %s%s%s", - NM_PRINT_FMT_QUOTED (priv->value, "\"", priv->value, "\"", "(none)"), - NM_PRINT_FMT_QUOTED (hostname, "\"", hostname, "\"", "(none)")); - g_free (priv->value); - priv->value = g_strdup (hostname); - _notify (self, PROP_HOSTNAME); - nm_dispatcher_call_hostname (NULL, NULL, NULL); + if (v_hostname) { + _set_hostname (self, g_variant_get_string (v_hostname, NULL)); + g_variant_unref (v_hostname); } - - g_variant_unref (v_hostname); } static void @@ -487,8 +505,6 @@ setup_hostname_file_monitors (NMHostnameManager *self) struct stat file_stat; GFile *file; - priv->value = nm_hostname_manager_get_hostname (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) @@ -526,7 +542,7 @@ setup_hostname_file_monitors (NMHostnameManager *self) } #endif - hostname_maybe_changed (self); + _set_hostname_read (self); } /*****************************************************************************/ @@ -539,11 +555,7 @@ get_property (GObject *object, guint prop_id, switch (prop_id) { case PROP_HOSTNAME: - g_value_take_string (value, nm_hostname_manager_get_hostname (self)); - - /* Don't ever pass NULL through D-Bus */ - if (!g_value_get_string (value)) - g_value_set_static_string (value, ""); + g_value_set_string (value, nm_hostname_manager_get_hostname (self)); break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); @@ -625,7 +637,7 @@ dispose (GObject *object) g_clear_object (&priv->dhcp_monitor); } - g_clear_pointer (&priv->value, g_free); + nm_clear_g_free (&priv->current_hostname); G_OBJECT_CLASS (nm_hostname_manager_parent_class)->dispose (object); } diff --git a/src/nm-hostname-manager.h b/src/nm-hostname-manager.h index 92702bbc1b..a837e9b92d 100644 --- a/src/nm-hostname-manager.h +++ b/src/nm-hostname-manager.h @@ -44,7 +44,9 @@ GType nm_hostname_manager_get_type (void); NMHostnameManager *nm_hostname_manager_get (void); -char *nm_hostname_manager_get_hostname (NMHostnameManager *self); +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); diff --git a/src/nm-manager.c b/src/nm-manager.c index 0d65e58537..a845321ab7 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -35,6 +35,7 @@ #include "devices/nm-device.h" #include "devices/nm-device-generic.h" #include "platform/nm-platform.h" +#include "nm-hostname-manager.h" #include "nm-rfkill-manager.h" #include "dhcp/nm-dhcp-manager.h" #include "settings/nm-settings.h" @@ -122,6 +123,8 @@ typedef struct { NMPolicy *policy; + NMHostnameManager *hostname_manager; + NMBusManager *dbus_mgr; struct { GDBusConnection *connection; @@ -132,7 +135,6 @@ typedef struct { NMCheckpointManager *checkpoint_mgr; NMSettings *settings; - char *hostname; RadioState radio_states[RFKILL_TYPE_MAX]; NMVpnManager *vpn_manager; @@ -211,7 +213,6 @@ NM_GOBJECT_PROPERTIES_DEFINE (NMManager, PROP_ALL_DEVICES, /* Not exported */ - PROP_HOSTNAME, PROP_SLEEPING, ); @@ -1404,38 +1405,17 @@ system_unmanaged_devices_changed_cb (NMSettings *settings, } static void -system_hostname_changed_cb (NMSettings *settings, - GParamSpec *pspec, - gpointer user_data) +hostname_changed_cb (NMHostnameManager *hostname_manager, + GParamSpec *pspec, + NMManager *self) { - NMManager *self = NM_MANAGER (user_data); NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); - char *hostname; - - g_object_get (priv->settings, - NM_SETTINGS_HOSTNAME, - &hostname, - NULL); - - /* nm_settings_get_hostname() does not return an empty hostname. */ - nm_assert (!hostname || *hostname); - - if (!hostname && !priv->hostname) - return; - if (hostname && priv->hostname && !strcmp (hostname, priv->hostname)) { - g_free (hostname); - return; - } - - /* realloc, to free possibly trailing data after NUL. */ - if (hostname) - hostname = g_realloc (hostname, strlen (hostname) + 1); + const char *hostname; - g_free (priv->hostname); - priv->hostname = hostname; - _notify (self, PROP_HOSTNAME); + hostname = nm_hostname_manager_get_hostname (priv->hostname_manager); - nm_dhcp_manager_set_default_hostname (nm_dhcp_manager_get (), priv->hostname); + nm_dispatcher_call_hostname (NULL, NULL, NULL); + nm_dhcp_manager_set_default_hostname (nm_dhcp_manager_get (), hostname); } /*****************************************************************************/ @@ -5090,7 +5070,7 @@ nm_manager_start (NMManager *self, GError **error) priv->net_enabled ? "enabled" : "disabled"); system_unmanaged_devices_changed_cb (priv->settings, NULL, self); - system_hostname_changed_cb (priv->settings, NULL, self); + hostname_changed_cb (priv->hostname_manager, NULL, self); /* Start device factories */ nm_device_factory_manager_load_factories (_register_device_factory, self); @@ -5986,12 +5966,15 @@ constructed (GObject *object) G_CALLBACK (settings_startup_complete_changed), self); g_signal_connect (priv->settings, "notify::" NM_SETTINGS_UNMANAGED_SPECS, G_CALLBACK (system_unmanaged_devices_changed_cb), self); - g_signal_connect (priv->settings, "notify::" NM_SETTINGS_HOSTNAME, - G_CALLBACK (system_hostname_changed_cb), self); g_signal_connect (priv->settings, NM_SETTINGS_SIGNAL_CONNECTION_ADDED, G_CALLBACK (connection_added_cb), self); g_signal_connect (priv->settings, NM_SETTINGS_SIGNAL_CONNECTION_UPDATED, G_CALLBACK (connection_updated_cb), self); + + priv->hostname_manager = g_object_ref (nm_hostname_manager_get ()); + g_signal_connect (priv->hostname_manager, "notify::" NM_HOSTNAME_MANAGER_HOSTNAME, + G_CALLBACK (hostname_changed_cb), self); + /* * Do not delete existing virtual devices to keep connectivity up. * Virtual devices are reused when NetworkManager is restarted. @@ -6188,9 +6171,6 @@ get_property (GObject *object, guint prop_id, case PROP_ACTIVATING_CONNECTION: nm_utils_g_value_set_object_path (value, priv->activating_connection); break; - case PROP_HOSTNAME: - g_value_set_string (value, priv->hostname); - break; case PROP_SLEEPING: g_value_set_boolean (value, priv->sleeping); break; @@ -6299,8 +6279,6 @@ dispose (GObject *object) g_clear_object (&priv->config); } - g_free (priv->hostname); - if (priv->policy) { g_signal_handlers_disconnect_by_func (priv->policy, policy_default_device_changed, manager); g_signal_handlers_disconnect_by_func (priv->policy, policy_activating_device_changed, manager); @@ -6310,12 +6288,16 @@ dispose (GObject *object) if (priv->settings) { g_signal_handlers_disconnect_by_func (priv->settings, settings_startup_complete_changed, manager); g_signal_handlers_disconnect_by_func (priv->settings, system_unmanaged_devices_changed_cb, manager); - g_signal_handlers_disconnect_by_func (priv->settings, system_hostname_changed_cb, manager); g_signal_handlers_disconnect_by_func (priv->settings, connection_added_cb, manager); g_signal_handlers_disconnect_by_func (priv->settings, connection_updated_cb, manager); g_clear_object (&priv->settings); } + if (priv->hostname_manager) { + g_signal_handlers_disconnect_by_func (priv->hostname_manager, hostname_changed_cb, manager); + g_clear_object (&priv->hostname_manager); + } + g_clear_object (&priv->vpn_manager); /* Unregister property filter */ @@ -6477,13 +6459,6 @@ nm_manager_class_init (NMManagerClass *manager_class) G_PARAM_READABLE | G_PARAM_STATIC_STRINGS); - /* Hostname is not exported over D-Bus */ - obj_properties[PROP_HOSTNAME] = - g_param_spec_string (NM_MANAGER_HOSTNAME, "", "", - NULL, - G_PARAM_READABLE | - G_PARAM_STATIC_STRINGS); - /* Sleeping is not exported over D-Bus */ obj_properties[PROP_SLEEPING] = g_param_spec_boolean (NM_MANAGER_SLEEPING, "", "", diff --git a/src/nm-manager.h b/src/nm-manager.h index 676fa995b6..fbbaffc314 100644 --- a/src/nm-manager.h +++ b/src/nm-manager.h @@ -54,7 +54,6 @@ #define NM_MANAGER_ALL_DEVICES "all-devices" /* Not exported */ -#define NM_MANAGER_HOSTNAME "hostname" #define NM_MANAGER_SLEEPING "sleeping" /* signals */ diff --git a/src/nm-policy.c b/src/nm-policy.c index 24b3cba8c4..36c05e1afa 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -587,7 +587,7 @@ static void update_system_hostname (NMPolicy *self, NMDevice *best4, NMDevice *best6, const char *msg) { NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self); - char *configured_hostname = NULL; + const char *configured_hostname; gs_free char *temp_hostname = NULL; const char *dhcp_hostname, *p; NMIP4Config *ip4_config; @@ -638,14 +638,12 @@ update_system_hostname (NMPolicy *self, NMDevice *best4, NMDevice *best6, const */ /* Try a persistent hostname first */ - g_object_get (G_OBJECT (priv->manager), NM_MANAGER_HOSTNAME, &configured_hostname, NULL); + configured_hostname = nm_hostname_manager_get_hostname (priv->hostname_manager); if (configured_hostname && nm_utils_is_specific_hostname (configured_hostname)) { _set_hostname (self, configured_hostname, "from system configuration"); priv->dhcp_hostname = FALSE; - g_free (configured_hostname); return; } - g_free (configured_hostname); /* Try automatically determined hostname from the best device's IP config */ if (!best4) @@ -1251,7 +1249,7 @@ process_secondaries (NMPolicy *self, } static void -hostname_changed (NMManager *manager, GParamSpec *pspec, gpointer user_data) +hostname_changed (NMHostnameManager *hostname_manager, GParamSpec *pspec, gpointer user_data) { NMPolicyPrivate *priv = user_data; NMPolicy *self = _PRIV_TO_SELF (priv); @@ -2350,7 +2348,8 @@ constructed (GObject *object) priv->resolver = g_resolver_get_default (); - g_signal_connect (priv->manager, "notify::" NM_MANAGER_HOSTNAME, (GCallback) hostname_changed, priv); + g_signal_connect (priv->hostname_manager, "notify::" NM_HOSTNAME_MANAGER_HOSTNAME, (GCallback) hostname_changed, priv); + g_signal_connect (priv->manager, "notify::" NM_MANAGER_SLEEPING, (GCallback) sleeping_changed, priv); g_signal_connect (priv->manager, "notify::" NM_MANAGER_NETWORKING_ENABLED, (GCallback) sleeping_changed, priv); g_signal_connect (priv->manager, NM_MANAGER_INTERNAL_DEVICE_ADDED, (GCallback) device_added, priv); @@ -2429,6 +2428,11 @@ dispose (GObject *object) g_clear_pointer (&priv->cur_hostname, g_free); g_clear_pointer (&priv->last_hostname, g_free); + if (priv->hostname_manager) { + g_signal_handlers_disconnect_by_data (priv->hostname_manager, priv); + g_clear_object (&priv->hostname_manager); + } + if (priv->settings) { g_signal_handlers_disconnect_by_data (priv->settings, priv); g_clear_object (&priv->settings); diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index 2e41f9c3a4..8db58c8d3a 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -156,7 +156,6 @@ typedef struct { gboolean started; gboolean startup_complete; - char *hostname; NMHostnameManager *hostname_manager; } NMSettingsPrivate; @@ -1794,27 +1793,12 @@ nm_settings_get_startup_complete (NMSettings *self) /*****************************************************************************/ -static void -_hostname_changed (NMSettings *self) -{ - NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self); - gs_free char *hostname = NULL; - - hostname = nm_hostname_manager_get_hostname (priv->hostname_manager); - - if (nm_streq0 (hostname, priv->hostname)) - return; - g_free (priv->hostname); - priv->hostname = g_steal_pointer (&hostname); - _notify (self, PROP_HOSTNAME); -} - static void _hostname_changed_cb (NMHostnameManager *hostname_manager, GParamSpec *pspec, gpointer user_data) { - _hostname_changed (NM_SETTINGS (user_data)); + _notify (user_data, PROP_HOSTNAME); } /*****************************************************************************/ @@ -1843,7 +1827,9 @@ nm_settings_start (NMSettings *self, GError **error) "notify::"NM_HOSTNAME_MANAGER_HOSTNAME, G_CALLBACK (_hostname_changed_cb), self); - _hostname_changed (self); + if (nm_hostname_manager_get_hostname (priv->hostname_manager)) + _notify (self, PROP_HOSTNAME); + return TRUE; } @@ -1870,10 +1856,10 @@ get_property (GObject *object, guint prop_id, g_value_take_boxed (value, (char **) g_ptr_array_free (array, FALSE)); break; case PROP_HOSTNAME: - if (priv->hostname) - g_value_set_string (value, priv->hostname); - else - g_value_set_static_string (value, ""); + g_value_set_string (value, + priv->hostname_manager + ? nm_hostname_manager_get_hostname (priv->hostname_manager) + : NULL); break; case PROP_CAN_MODIFY: g_value_set_boolean (value, !!get_plugin (self, NM_SETTINGS_PLUGIN_CAP_MODIFY_CONNECTIONS)); @@ -1957,8 +1943,6 @@ finalize (GObject *object) g_slist_free_full (priv->plugins, g_object_unref); - g_free (priv->hostname); - g_clear_object (&priv->config); G_OBJECT_CLASS (nm_settings_parent_class)->finalize (object); -- cgit v1.2.1 From 9f4a5b85b6d3dba10dcf51b325113bd2b1068030 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 23 Apr 2017 18:35:24 +0200 Subject: hostname: change logging of hostname-mode Don't log with level . It's not important enough and only noise. Also, move the logging line outside of nm_policy_init(). At that point, the policy instance is not yet initialized (much), so we shouldn't use the logging macro _LOGD(). --- src/nm-policy.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index 36c05e1afa..19193e9cf6 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -2241,6 +2241,15 @@ nm_policy_get_activating_ip6_device (NMPolicy *self) /*****************************************************************************/ +NM_UTILS_LOOKUP_STR_DEFINE_STATIC (_hostname_mode_to_string, NMPolicyHostnameMode, + NM_UTILS_LOOKUP_DEFAULT_NM_ASSERT ("unknown"), + NM_UTILS_LOOKUP_STR_ITEM (NM_POLICY_HOSTNAME_MODE_NONE, "none"), + NM_UTILS_LOOKUP_STR_ITEM (NM_POLICY_HOSTNAME_MODE_DHCP, "dhcp"), + NM_UTILS_LOOKUP_STR_ITEM (NM_POLICY_HOSTNAME_MODE_FULL, "full"), +); + +/*****************************************************************************/ + static void get_property (GObject *object, guint prop_id, GValue *value, GParamSpec *pspec) @@ -2314,7 +2323,6 @@ nm_policy_init (NMPolicy *self) else /* default - full mode */ priv->hostname_mode = NM_POLICY_HOSTNAME_MODE_FULL; - _LOGI (LOGD_DNS, "hostname management mode: %s", hostname_mode ? hostname_mode : "default"); priv->devices = g_hash_table_new (NULL, NULL); priv->ip6_prefix_delegations = g_array_new (FALSE, FALSE, sizeof (IP6PrefixDelegation)); g_array_set_clear_func (priv->ip6_prefix_delegations, clear_ip6_prefix_delegation); @@ -2364,6 +2372,8 @@ constructed (GObject *object) g_signal_connect (priv->settings, NM_SETTINGS_SIGNAL_AGENT_REGISTERED, (GCallback) secret_agent_registered, priv); G_OBJECT_CLASS (nm_policy_parent_class)->constructed (object); + + _LOGD (LOGD_DNS, "hostname-mode: %s", _hostname_mode_to_string (priv->hostname_mode)); } NMPolicy * -- cgit v1.2.1 From 5819b0de8e1a0f7ec5101801ef4686a2b8d8c0dd Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 23 Apr 2017 19:24:21 +0200 Subject: policy: add lookup_by_address() function for g_resolver_lookup_by_address_async() --- src/nm-policy.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index 19193e9cf6..72253addbf 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -583,6 +583,19 @@ lookup_callback (GObject *source, _set_hostname (self, NULL, error->message); } +static void +lookup_by_address (NMPolicy *self) +{ + NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self); + + nm_clear_g_cancellable (&priv->lookup_cancellable); + priv->lookup_cancellable = g_cancellable_new (); + g_resolver_lookup_by_address_async (priv->resolver, + priv->lookup_addr, + priv->lookup_cancellable, + lookup_callback, self); +} + static void update_system_hostname (NMPolicy *self, NMDevice *best4, NMDevice *best6, const char *msg) { @@ -754,11 +767,7 @@ update_system_hostname (NMPolicy *self, NMDevice *best4, NMDevice *best6, const return; } - priv->lookup_cancellable = g_cancellable_new (); - g_resolver_lookup_by_address_async (priv->resolver, - priv->lookup_addr, - priv->lookup_cancellable, - lookup_callback, self); + lookup_by_address (self); } static void @@ -2108,11 +2117,7 @@ dns_config_changed (NMDnsManager *dns_manager, gpointer user_data) (str = g_inet_address_to_string (priv->lookup_addr))); g_free (str); - priv->lookup_cancellable = g_cancellable_new (); - g_resolver_lookup_by_address_async (priv->resolver, - priv->lookup_addr, - priv->lookup_cancellable, - lookup_callback, self); + lookup_by_address (self); } } -- cgit v1.2.1 From f0a229c1e2abf936eec6b5771f97856fbb8825c7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 23 Apr 2017 19:30:07 +0200 Subject: policy: namespace fields related to DNS reverse lookup for hostname --- src/nm-policy.c | 50 ++++++++++++++++++++++++++------------------------ 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index 72253addbf..bb33ecde26 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -79,9 +79,12 @@ typedef struct { NMDevice *default_device4, *activating_device4; NMDevice *default_device6, *activating_device6; - GResolver *resolver; - GInetAddress *lookup_addr; - GCancellable *lookup_cancellable; + struct { + GInetAddress *addr; + GResolver *resolver; + GCancellable *cancellable; + } lookup; + NMDnsManager *dns_manager; gulong config_changed_id; @@ -511,7 +514,7 @@ _set_hostname (NMPolicy *self, * restart the reverse lookup thread later. */ if (new_hostname) - g_clear_object (&priv->lookup_addr); + g_clear_object (&priv->lookup.addr); /* Update the DNS only if the hostname is actually * going to change. @@ -575,7 +578,7 @@ lookup_callback (GObject *source, self = user_data; priv = NM_POLICY_GET_PRIVATE (self); - g_clear_object (&priv->lookup_cancellable); + g_clear_object (&priv->lookup.cancellable); if (hostname) _set_hostname (self, hostname, "from address lookup"); @@ -588,11 +591,11 @@ lookup_by_address (NMPolicy *self) { NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self); - nm_clear_g_cancellable (&priv->lookup_cancellable); - priv->lookup_cancellable = g_cancellable_new (); - g_resolver_lookup_by_address_async (priv->resolver, - priv->lookup_addr, - priv->lookup_cancellable, + nm_clear_g_cancellable (&priv->lookup.cancellable); + priv->lookup.cancellable = g_cancellable_new (); + g_resolver_lookup_by_address_async (priv->lookup.resolver, + priv->lookup.addr, + priv->lookup.cancellable, lookup_callback, self); } @@ -616,7 +619,7 @@ update_system_hostname (NMPolicy *self, NMDevice *best4, NMDevice *best6, const _LOGT (LOGD_DNS, "set-hostname: updating hostname (%s)", msg); - nm_clear_g_cancellable (&priv->lookup_cancellable); + nm_clear_g_cancellable (&priv->lookup.cancellable); /* Check if the hostname was set externally to NM, so that in that case * we can avoid to fallback to the one we got when we started. @@ -751,15 +754,15 @@ update_system_hostname (NMPolicy *self, NMDevice *best4, NMDevice *best6, const const NMPlatformIP4Address *addr4; addr4 = nm_ip4_config_get_address (ip4_config, 0); - g_clear_object (&priv->lookup_addr); - priv->lookup_addr = g_inet_address_new_from_bytes ((guint8 *) &addr4->address, + g_clear_object (&priv->lookup.addr); + priv->lookup.addr = g_inet_address_new_from_bytes ((guint8 *) &addr4->address, G_SOCKET_FAMILY_IPV4); } else if (ip6_config && nm_ip6_config_get_num_addresses (ip6_config) > 0) { const NMPlatformIP6Address *addr6; addr6 = nm_ip6_config_get_address (ip6_config, 0); - g_clear_object (&priv->lookup_addr); - priv->lookup_addr = g_inet_address_new_from_bytes ((guint8 *) &addr6->address, + g_clear_object (&priv->lookup.addr); + priv->lookup.addr = g_inet_address_new_from_bytes ((guint8 *) &addr6->address, G_SOCKET_FAMILY_IPV6); } else { /* No valid IP config; fall back to localhost.localdomain */ @@ -2098,10 +2101,10 @@ dns_config_changed (NMDnsManager *dns_manager, gpointer user_data) * (race in updating DNS and doing the reverse lookup). */ - nm_clear_g_cancellable (&priv->lookup_cancellable); + nm_clear_g_cancellable (&priv->lookup.cancellable); /* Re-start the hostname lookup thread if we don't have hostname yet. */ - if (priv->lookup_addr) { + if (priv->lookup.addr) { char *str = NULL; gs_free char *hostname = NULL; @@ -2109,12 +2112,12 @@ dns_config_changed (NMDnsManager *dns_manager, gpointer user_data) if ( _get_hostname (self, &hostname) && nm_utils_is_specific_hostname (hostname) && !nm_streq0 (hostname, priv->last_hostname)) { - g_clear_object (&priv->lookup_addr); + g_clear_object (&priv->lookup.addr); return; } _LOGD (LOGD_DNS, "restarting reverse-lookup thread for address %s", - (str = g_inet_address_to_string (priv->lookup_addr))); + (str = g_inet_address_to_string (priv->lookup.addr))); g_free (str); lookup_by_address (self); @@ -2359,7 +2362,7 @@ constructed (GObject *object) priv->config_changed_id = g_signal_connect (priv->dns_manager, NM_DNS_MANAGER_CONFIG_CHANGED, G_CALLBACK (dns_config_changed), self); - priv->resolver = g_resolver_get_default (); + priv->lookup.resolver = g_resolver_get_default (); g_signal_connect (priv->hostname_manager, "notify::" NM_HOSTNAME_MANAGER_HOSTNAME, (GCallback) hostname_changed, priv); @@ -2402,10 +2405,9 @@ dispose (GObject *object) GHashTableIter h_iter; NMDevice *device; - nm_clear_g_cancellable (&priv->lookup_cancellable); - - g_clear_object (&priv->lookup_addr); - g_clear_object (&priv->resolver); + nm_clear_g_cancellable (&priv->lookup.cancellable); + g_clear_object (&priv->lookup.addr); + g_clear_object (&priv->lookup.resolver); while (priv->pending_activation_checks) activate_data_free (priv->pending_activation_checks->data); -- cgit v1.2.1 From 25c5a96da7f26df14131bad44703a44eb89c26ff Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 23 Apr 2017 19:54:39 +0200 Subject: policy: drop out argument from _get_hostname() It serves no purpose, because the function always allocates a new result and returns it. It would make sense, if the output string would only be cloned when we need to allocate a new string. But since that optimization is not done, the argument serves no purpose. --- src/nm-policy.c | 49 ++++++++++++++++++++++--------------------------- 1 file changed, 22 insertions(+), 27 deletions(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index bb33ecde26..3b27b2063d 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -449,50 +449,46 @@ settings_set_hostname_cb (const char *hostname, #define HOST_NAME_BUFSIZE (HOST_NAME_MAX + 2) static char * -_get_hostname (NMPolicy *self, char **hostname) +_get_hostname (NMPolicy *self) { NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self); - char *buf; - - g_assert (hostname && *hostname == NULL); + char *hostname = NULL; /* If there is an in-progress hostname change, return * the last hostname set as would be set soon... */ if (priv->changing_hostname) { _LOGT (LOGD_DNS, "get-hostname: \"%s\" (last on set)", priv->last_hostname); - *hostname = g_strdup (priv->last_hostname); - return *hostname; + return g_strdup (priv->last_hostname); } /* try to get the hostname via dbus... */ - if (nm_hostname_manager_get_transient_hostname (priv->hostname_manager, hostname)) { - _LOGT (LOGD_DNS, "get-hostname: \"%s\" (from dbus)", *hostname); - return *hostname; + if (nm_hostname_manager_get_transient_hostname (priv->hostname_manager, &hostname)) { + _LOGT (LOGD_DNS, "get-hostname: \"%s\" (from dbus)", hostname); + return hostname; } /* ...or retrieve it by yourself */ - buf = g_malloc (HOST_NAME_BUFSIZE); - if (gethostname (buf, HOST_NAME_BUFSIZE -1) != 0) { + hostname = g_malloc (HOST_NAME_BUFSIZE); + if (gethostname (hostname, HOST_NAME_BUFSIZE -1) != 0) { int errsv = errno; _LOGT (LOGD_DNS, "get-hostname: couldn't get the system hostname: (%d) %s", errsv, g_strerror (errsv)); - g_free (buf); + g_free (hostname); return NULL; } /* the name may be truncated... */ - buf[HOST_NAME_BUFSIZE - 1] = '\0'; - if (strlen (buf) >= HOST_NAME_BUFSIZE -1) { - _LOGT (LOGD_DNS, "get-hostname: system hostname too long: \"%s\"", buf); - g_free (buf); + hostname[HOST_NAME_BUFSIZE - 1] = '\0'; + if (strlen (hostname) >= HOST_NAME_BUFSIZE -1) { + _LOGT (LOGD_DNS, "get-hostname: system hostname too long: \"%s\"", hostname); + g_free (hostname); return NULL; } - _LOGT (LOGD_DNS, "get-hostname: \"%s\"", buf); - *hostname = buf; - return *hostname; + _LOGT (LOGD_DNS, "get-hostname: \"%s\"", hostname); + return hostname; } static void @@ -540,7 +536,7 @@ _set_hostname (NMPolicy *self, name = new_hostname; /* Don't set the hostname if it isn't actually changing */ - if ( _get_hostname (self, &old_hostname) + if ( (old_hostname = _get_hostname (self)) && (nm_streq (name, old_hostname))) { _LOGT (LOGD_DNS, "set-hostname: hostname already set to '%s' (%s)", name, msg); return; @@ -624,7 +620,7 @@ update_system_hostname (NMPolicy *self, NMDevice *best4, NMDevice *best6, const /* Check if the hostname was set externally to NM, so that in that case * we can avoid to fallback to the one we got when we started. * Consider "not specific" hostnames as equal. */ - if ( _get_hostname (self, &temp_hostname) + if ( (temp_hostname = _get_hostname (self)) && !nm_streq0 (temp_hostname, priv->last_hostname) && ( nm_utils_is_specific_hostname (temp_hostname) || nm_utils_is_specific_hostname (priv->last_hostname))) { @@ -636,10 +632,9 @@ update_system_hostname (NMPolicy *self, NMDevice *best4, NMDevice *best6, const if (!nm_streq0 (temp_hostname, priv->orig_hostname)) { /* Update original (fallback) hostname */ g_free (priv->orig_hostname); - if (nm_utils_is_specific_hostname (temp_hostname)) { - priv->orig_hostname = temp_hostname; - temp_hostname = NULL; - } else + if (nm_utils_is_specific_hostname (temp_hostname)) + priv->orig_hostname = g_steal_pointer (&temp_hostname); + else priv->orig_hostname = NULL; } } @@ -2109,7 +2104,7 @@ dns_config_changed (NMDnsManager *dns_manager, gpointer user_data) gs_free char *hostname = NULL; /* Check if the hostname was externally set */ - if ( _get_hostname (self, &hostname) + if ( (hostname = _get_hostname (self)) && nm_utils_is_specific_hostname (hostname) && !nm_streq0 (hostname, priv->last_hostname)) { g_clear_object (&priv->lookup.addr); @@ -2344,7 +2339,7 @@ constructed (GObject *object) char *hostname = NULL; /* Grab hostname on startup and use that if nothing provides one */ - if (_get_hostname (self, &hostname)) { + if ((hostname = _get_hostname (self))) { /* init last_hostname */ priv->last_hostname = hostname; -- cgit v1.2.1 From af5b86aa1ec59f9215604e55e2d3ee0026d718ff Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 23 Apr 2017 20:31:31 +0200 Subject: policy: log policy's orig_hostname --- src/dns/nm-dns-manager.c | 1 + src/nm-policy.c | 11 +++++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/dns/nm-dns-manager.c b/src/dns/nm-dns-manager.c index 1f6c2eede4..cc9be29e7f 100644 --- a/src/dns/nm-dns-manager.c +++ b/src/dns/nm-dns-manager.c @@ -1435,6 +1435,7 @@ nm_dns_manager_set_initial_hostname (NMDnsManager *self, { NMDnsManagerPrivate *priv = NM_DNS_MANAGER_GET_PRIVATE (self); + g_free (priv->hostname); priv->hostname = g_strdup (hostname); } diff --git a/src/nm-policy.c b/src/nm-policy.c index 3b27b2063d..2d17bf3af0 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -629,13 +629,14 @@ update_system_hostname (NMPolicy *self, NMDevice *best4, NMDevice *best6, const temp_hostname); priv->dhcp_hostname = FALSE; + if (!nm_utils_is_specific_hostname (temp_hostname)) + nm_clear_g_free (&temp_hostname); if (!nm_streq0 (temp_hostname, priv->orig_hostname)) { /* Update original (fallback) hostname */ g_free (priv->orig_hostname); - if (nm_utils_is_specific_hostname (temp_hostname)) - priv->orig_hostname = g_steal_pointer (&temp_hostname); - else - priv->orig_hostname = NULL; + priv->orig_hostname = g_steal_pointer (&temp_hostname); + _LOGT (LOGD_DNS, "hostname-original: update to %s%s%s", + NM_PRINT_FMT_QUOTE_STRING (priv->orig_hostname)); } } @@ -2347,6 +2348,8 @@ constructed (GObject *object) if (nm_utils_is_specific_hostname (hostname)) priv->orig_hostname = g_strdup (hostname); } + _LOGT (LOGD_DNS, "hostname-original: set to %s%s%s", + NM_PRINT_FMT_QUOTE_STRING (priv->orig_hostname)); priv->firewall_manager = g_object_ref (nm_firewall_manager_get ()); g_signal_connect (priv->firewall_manager, NM_FIREWALL_MANAGER_STATE_CHANGED, -- cgit v1.2.1