From 9bb96b00a5df75d7d07daefd7b9fa5e877042a79 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Wed, 9 Dec 2015 11:03:27 -0600 Subject: [PATCH] adsl: look up ATM index before construction Fixes a crash if we can't read the ATM index. We need the ATM index, and we can't do anything with the device before we have it, so don't bother creating one if we we can't get it. NetworkManager[9662]: [1449678770.705541] [nm-device-adsl.c:607] constructor(): (atmtcp0): error reading ATM device index (NetworkManager:9662): GLib-GObject-CRITICAL **: object NMDeviceAdsl 0x1e8f880 finalized while still in-construction (NetworkManager:9662): GLib-GObject-CRITICAL **: Custom constructor for class NMDeviceAdsl returned NULL (which is invalid). Please use GInitable instead. ** NetworkManager-adsl:ERROR:nm-atm-manager.c:121:adsl_add: assertion failed: (device) --- src/devices/adsl/nm-atm-manager.c | 43 +++++++++---- src/devices/adsl/nm-device-adsl.c | 101 +++++++++++++++++------------- src/devices/adsl/nm-device-adsl.h | 6 +- 3 files changed, 93 insertions(+), 57 deletions(-) diff --git a/src/devices/adsl/nm-atm-manager.c b/src/devices/adsl/nm-atm-manager.c index b29bc7b19a..d3f1423f6d 100644 --- a/src/devices/adsl/nm-atm-manager.c +++ b/src/devices/adsl/nm-atm-manager.c @@ -29,6 +29,7 @@ #include "nm-setting-adsl.h" #include "nm-device-adsl.h" #include "nm-device-factory.h" +#include "nm-platform.h" typedef struct { GUdevClient *client; @@ -102,6 +103,8 @@ adsl_add (NMAtmManager *self, GUdevDevice *udev_device) NMAtmManagerPrivate *priv = NM_ATM_MANAGER_GET_PRIVATE (self); const char *ifname, *sysfs_path = NULL; char *driver = NULL; + gs_free char *atm_index_path = NULL; + int atm_index; NMDevice *device; g_return_if_fail (udev_device != NULL); @@ -114,20 +117,34 @@ adsl_add (NMAtmManager *self, GUdevDevice *udev_device) nm_log_dbg (LOGD_HW, "(%s): found ATM device", ifname); - if (dev_get_attrs (udev_device, &sysfs_path, &driver)) { - g_assert (sysfs_path); - - device = nm_device_adsl_new (sysfs_path, ifname, driver); - g_assert (device); - - priv->devices = g_slist_prepend (priv->devices, device); - g_object_weak_ref (G_OBJECT (device), device_destroyed, self); - - g_signal_emit_by_name (self, NM_DEVICE_FACTORY_DEVICE_ADDED, device); - g_object_unref (device); - - g_free (driver); + atm_index_path = g_strdup_printf ("/sys/class/atm/%s/atmindex", + ASSERT_VALID_PATH_COMPONENT (ifname)); + atm_index = (int) nm_platform_sysctl_get_int_checked (NM_PLATFORM_GET, + atm_index_path, + 10, 0, G_MAXINT, + -1); + if (atm_index < 0) { + nm_log_warn (LOGD_HW, "(%s): failed to get ATM index", ifname); + return; } + + if (!dev_get_attrs (udev_device, &sysfs_path, &driver)) { + nm_log_warn (LOGD_HW, "(%s): failed to get ATM attributes", ifname); + return; + } + + g_assert (sysfs_path); + + device = nm_device_adsl_new (sysfs_path, ifname, driver, atm_index); + g_assert (device); + + priv->devices = g_slist_prepend (priv->devices, device); + g_object_weak_ref (G_OBJECT (device), device_destroyed, self); + + g_signal_emit_by_name (self, NM_DEVICE_FACTORY_DEVICE_ADDED, device); + g_object_unref (device); + + g_free (driver); } static void diff --git a/src/devices/adsl/nm-device-adsl.c b/src/devices/adsl/nm-device-adsl.c index a1cec87870..f90a5c3a99 100644 --- a/src/devices/adsl/nm-device-adsl.c +++ b/src/devices/adsl/nm-device-adsl.c @@ -35,7 +35,6 @@ #include "nm-default.h" #include "nm-device-adsl.h" #include "nm-device-private.h" -#include "NetworkManagerUtils.h" #include "nm-enum-types.h" #include "nm-platform.h" @@ -52,6 +51,13 @@ G_DEFINE_TYPE (NMDeviceAdsl, nm_device_adsl, NM_TYPE_DEVICE) #define NM_DEVICE_ADSL_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), NM_TYPE_DEVICE_ADSL, NMDeviceAdslPrivate)) +enum { + PROP_0, + PROP_ATM_INDEX, + + LAST_PROP +}; + /**********************************************/ typedef struct { @@ -549,63 +555,35 @@ carrier_update_cb (gpointer user_data) NMDevice * nm_device_adsl_new (const char *udi, const char *iface, - const char *driver) + const char *driver, + int atm_index) { g_return_val_if_fail (udi != NULL, NULL); + g_return_val_if_fail (atm_index >= 0, NULL); return (NMDevice *) g_object_new (NM_TYPE_DEVICE_ADSL, NM_DEVICE_UDI, udi, NM_DEVICE_IFACE, iface, NM_DEVICE_DRIVER, driver, + NM_DEVICE_ADSL_ATM_INDEX, atm_index, NM_DEVICE_TYPE_DESC, "ADSL", NM_DEVICE_DEVICE_TYPE, NM_DEVICE_TYPE_ADSL, NULL); } -static int -get_atm_index (const char *iface) +static void +constructed (GObject *object) { - char *path; - int idx; + NMDeviceAdsl *self = NM_DEVICE_ADSL (object); + NMDeviceAdslPrivate *priv = NM_DEVICE_ADSL_GET_PRIVATE (self); - path = g_strdup_printf ("/sys/class/atm/%s/atmindex", - ASSERT_VALID_PATH_COMPONENT (iface)); - idx = (int) nm_platform_sysctl_get_int_checked (NM_PLATFORM_GET, path, 10, 0, G_MAXINT, -1); - g_free (path); + G_OBJECT_CLASS (nm_device_adsl_parent_class)->constructed (object); - return idx; -} + priv->carrier_poll_id = g_timeout_add_seconds (5, carrier_update_cb, self); -static GObject* -constructor (GType type, - guint n_construct_params, - GObjectConstructParam *construct_params) -{ - GObject *object; - NMDeviceAdsl *self; - NMDeviceAdslPrivate *priv; + _LOGD (LOGD_ADSL, "ATM device index %d", priv->atm_index); - object = G_OBJECT_CLASS (nm_device_adsl_parent_class)->constructor (type, - n_construct_params, - construct_params); - if (!object) - return NULL; - - self = NM_DEVICE_ADSL (object); - priv = NM_DEVICE_ADSL_GET_PRIVATE (object); - - priv->atm_index = get_atm_index (nm_device_get_iface (NM_DEVICE (object))); - if (priv->atm_index < 0) { - _LOGE (LOGD_ADSL, "error reading ATM device index"); - g_object_unref (object); - return NULL; - } else - _LOGD (LOGD_ADSL, "ATM device index %d", priv->atm_index); - - /* Poll the carrier */ - priv->carrier_poll_id = g_timeout_add_seconds (5, carrier_update_cb, object); - - return object; + g_return_if_fail (priv->atm_index >= 0); } static void @@ -618,6 +596,35 @@ dispose (GObject *object) G_OBJECT_CLASS (nm_device_adsl_parent_class)->dispose (object); } +static void +get_property (GObject *object, guint prop_id, + GValue *value, GParamSpec *pspec) +{ + switch (prop_id) { + case PROP_ATM_INDEX: + g_value_set_int (value, NM_DEVICE_ADSL_GET_PRIVATE (object)->atm_index); + break; + default: + G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); + break; + } +} + +static void +set_property (GObject *object, guint prop_id, + const GValue *value, GParamSpec *pspec) +{ + switch (prop_id) { + case PROP_ATM_INDEX: + /* construct only */ + NM_DEVICE_ADSL_GET_PRIVATE (object)->atm_index = g_value_get_int (value); + break; + default: + G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); + break; + } +} + static void nm_device_adsl_init (NMDeviceAdsl *self) { @@ -631,8 +638,10 @@ nm_device_adsl_class_init (NMDeviceAdslClass *klass) g_type_class_add_private (object_class, sizeof (NMDeviceAdslPrivate)); - object_class->constructor = constructor; + object_class->constructed = constructed; object_class->dispose = dispose; + object_class->get_property = get_property; + object_class->set_property = set_property; parent_class->get_generic_capabilities = get_generic_capabilities; @@ -643,6 +652,14 @@ nm_device_adsl_class_init (NMDeviceAdslClass *klass) parent_class->act_stage3_ip4_config_start = act_stage3_ip4_config_start; parent_class->deactivate = deactivate; + /* properties */ + g_object_class_install_property + (object_class, PROP_ATM_INDEX, + g_param_spec_int (NM_DEVICE_ADSL_ATM_INDEX, "", "", + -1, G_MAXINT, -1, + G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY | + G_PARAM_STATIC_STRINGS)); + nm_exported_object_class_add_interface (NM_EXPORTED_OBJECT_CLASS (klass), NMDBUS_TYPE_DEVICE_ADSL_SKELETON, NULL); diff --git a/src/devices/adsl/nm-device-adsl.h b/src/devices/adsl/nm-device-adsl.h index 3587bfc75f..efcdaf6980 100644 --- a/src/devices/adsl/nm-device-adsl.h +++ b/src/devices/adsl/nm-device-adsl.h @@ -35,20 +35,22 @@ G_BEGIN_DECLS #define NM_IS_DEVICE_ADSL_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), NM_TYPE_DEVICE_ADSL)) #define NM_DEVICE_ADSL_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), NM_TYPE_DEVICE_ADSL, NMDeviceAdslClass)) +#define NM_DEVICE_ADSL_ATM_INDEX "atm-index" + typedef struct { NMDevice parent; } NMDeviceAdsl; typedef struct { NMDeviceClass parent; - } NMDeviceAdslClass; GType nm_device_adsl_get_type (void); NMDevice *nm_device_adsl_new (const char *udi, const char *iface, - const char *driver); + const char *driver, + int atm_index); G_END_DECLS