From a3d96601876e5dc34e68bf6777c770f83cac19fb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 6 Sep 2016 10:56:32 +0200 Subject: [PATCH 01/11] logging: don't round subsecond part in logging timestamp tv.tv_usec is guaranteed to have less then 6 digits, however rounding it up we might reach 1000000 and thus the value becomes mis-aligned. To round correctly, we would have to carry over a potential overflow to the seconds. But that seems too much effort for little gain. Just truncate the value. (cherry picked from commit c1b4b99a3c758f320c369a8daadb219eeb50ee83) (cherry picked from commit 99e30bdf700220e98db76602645a9844360e3fab) --- src/nm-logging.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nm-logging.c b/src/nm-logging.c index 3db8d20..6ecc160 100644 --- a/src/nm-logging.c +++ b/src/nm-logging.c @@ -512,7 +512,7 @@ _nm_log_impl (const char *file, va_end (args); g_get_current_time (&tv); - nm_sprintf_buf (s_buf_timestamp, " [%ld.%04ld]", tv.tv_sec, (tv.tv_usec + 50) / 100); + nm_sprintf_buf (s_buf_timestamp, " [%ld.%04ld]", tv.tv_sec, tv.tv_usec / 100); switch (global.log_backend) { #if SYSTEMD_JOURNAL -- 2.7.4 From b3bf5c5a771e1f105037323daa9f4e4696bce44c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 5 Sep 2016 14:12:41 +0200 Subject: [PATCH 02/11] shared: add NM_MIN()/NM_MAX() macros to replace glib's MIN()/MAX() (cherry picked from commit b2016fd2a52b82d45324526c965e7545d026cebe) (cherry picked from commit 811aaead4ca6f2f815f49b7353fa7a88554dca42) --- shared/nm-utils/nm-macros-internal.h | 44 ++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/shared/nm-utils/nm-macros-internal.h b/shared/nm-utils/nm-macros-internal.h index 73075c6..8811c91 100644 --- a/shared/nm-utils/nm-macros-internal.h +++ b/shared/nm-utils/nm-macros-internal.h @@ -526,6 +526,50 @@ nm_strcmp_p_with_data (gconstpointer a, gconstpointer b, gpointer user_data) /*****************************************************************************/ +/* Taken from systemd's UNIQ_T and UNIQ macros. */ + +#define NM_UNIQ_T(x, uniq) G_PASTE(__unique_prefix_, G_PASTE(x, uniq)) +#define NM_UNIQ __COUNTER__ + +/*****************************************************************************/ + +/* glib's MIN()/MAX() macros don't have function-like behavior, in that they evaluate + * the argument possibly twice. + * + * Taken from systemd's MIN()/MAX() macros. */ + +#define NM_MIN(a, b) __NM_MIN(NM_UNIQ, a, NM_UNIQ, b) +#define __NM_MIN(aq, a, bq, b) \ + ({ \ + typeof (a) NM_UNIQ_T(A, aq) = (a); \ + typeof (b) NM_UNIQ_T(B, bq) = (b); \ + ((NM_UNIQ_T(A, aq) < NM_UNIQ_T(B, bq)) ? NM_UNIQ_T(A, aq) : NM_UNIQ_T(B, bq)); \ + }) + +#define NM_MAX(a, b) __NM_MAX(NM_UNIQ, a, NM_UNIQ, b) +#define __NM_MAX(aq, a, bq, b) \ + ({ \ + typeof (a) NM_UNIQ_T(A, aq) = (a); \ + typeof (b) NM_UNIQ_T(B, bq) = (b); \ + ((NM_UNIQ_T(A, aq) > NM_UNIQ_T(B, bq)) ? NM_UNIQ_T(A, aq) : NM_UNIQ_T(B, bq)); \ + }) + +#define NM_CLAMP(x, low, high) __NM_CLAMP(NM_UNIQ, x, NM_UNIQ, low, NM_UNIQ, high) +#define __NM_CLAMP(xq, x, lowq, low, highq, high) \ + ({ \ + typeof(x)NM_UNIQ_T(X,xq) = (x); \ + typeof(low) NM_UNIQ_T(LOW,lowq) = (low); \ + typeof(high) NM_UNIQ_T(HIGH,highq) = (high); \ + \ + ( (NM_UNIQ_T(X,xq) > NM_UNIQ_T(HIGH,highq)) \ + ? NM_UNIQ_T(HIGH,highq) \ + : (NM_UNIQ_T(X,xq) < NM_UNIQ_T(LOW,lowq)) \ + ? NM_UNIQ_T(LOW,lowq) \ + : NM_UNIQ_T(X,xq)); \ + }) + +/*****************************************************************************/ + static inline guint nm_encode_version (guint major, guint minor, guint micro) { /* analog to the preprocessor macro NM_ENCODE_VERSION(). */ -- 2.7.4 From d0b31408a5e1eeb7c3e09e3e79a90cf09c80b934 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 5 Sep 2016 16:49:50 +0200 Subject: [PATCH 03/11] shared: add _NM_GET_PRIVATE() macro (cherry picked from commit 2cae9ba348ed6ea4d41ebd714d8c55f4d49feae9) (cherry picked from commit 5bac57496c0ee458456f5cf68560263cea6c23de) --- shared/nm-utils/nm-macros-internal.h | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/shared/nm-utils/nm-macros-internal.h b/shared/nm-utils/nm-macros-internal.h index 8811c91..5b020e9 100644 --- a/shared/nm-utils/nm-macros-internal.h +++ b/shared/nm-utils/nm-macros-internal.h @@ -356,6 +356,24 @@ _notify (obj_type *obj, _PropertyEnums prop) \ /*****************************************************************************/ +#define __NM_GET_PRIVATE(self, type, is_check, result_cmd) \ + ({ \ + /* preserve the const-ness of self. Unfortunately, that + * way, @self cannot be a void pointer */ \ + typeof (self) _self = (self); \ + \ + /* Get compiler error if variable is of wrong type */ \ + _nm_unused const type *_self2 = (_self); \ + \ + nm_assert (is_check (_self)); \ + ( result_cmd ); \ + }) + +#define _NM_GET_PRIVATE(self, type, is_check) __NM_GET_PRIVATE(self, type, is_check, &_self->_priv) +#define _NM_GET_PRIVATE_PTR(self, type, is_check) __NM_GET_PRIVATE(self, type, is_check, _self->_priv) + +/*****************************************************************************/ + static inline gpointer nm_g_object_ref (gpointer obj) { -- 2.7.4 From 24d94264bdc6c036e732e201b167c1c7d20794e1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 5 Sep 2016 16:55:07 +0200 Subject: [PATCH 04/11] core: use _NM_GET_PRIVATE() macros (cherry picked from commit cdf6ad40572f23be6f8b6971bd57b1002ffb9aaf) (cherry picked from commit 3940d63a7e6bff088bb3fb5e81c8cb2792b19b3a) --- src/devices/nm-device-ethernet.c | 13 +------------ src/devices/nm-device-veth.c | 13 +------------ src/devices/nm-device.c | 31 ++++++++++--------------------- src/devices/nm-device.h | 2 +- src/devices/wifi/nm-device-wifi.c | 13 +------------ src/devices/wifi/nm-wifi-ap.c | 13 +------------ src/dns-manager/nm-dns-manager.c | 13 +------------ src/nm-auth-subject.c | 13 +------------ src/nm-checkpoint.c | 15 ++------------- src/nm-ip4-config.c | 13 +------------ src/nm-ip6-config.c | 13 +------------ src/nm-manager.c | 15 ++------------- src/rdisc/nm-lndp-rdisc.c | 13 +------------ src/rdisc/nm-rdisc.c | 13 +------------ src/vpn-manager/nm-vpn-connection.c | 13 +------------ 15 files changed, 26 insertions(+), 180 deletions(-) diff --git a/src/devices/nm-device-ethernet.c b/src/devices/nm-device-ethernet.c index 90d472d..b213a0c 100644 --- a/src/devices/nm-device-ethernet.c +++ b/src/devices/nm-device-ethernet.c @@ -127,18 +127,7 @@ NM_GOBJECT_PROPERTIES_DEFINE (NMDeviceEthernet, G_DEFINE_TYPE (NMDeviceEthernet, nm_device_ethernet, NM_TYPE_DEVICE) -#define NM_DEVICE_ETHERNET_GET_PRIVATE(self) \ - ({ \ - /* preserve the const-ness of self. Unfortunately, that - * way, @self cannot be a void pointer */ \ - typeof (self) _self = (self); \ - \ - /* Get compiler error if variable is of wrong type */ \ - _nm_unused const NMDeviceEthernet *_self2 = (_self); \ - \ - nm_assert (NM_IS_DEVICE_ETHERNET (_self)); \ - _self->_priv; \ - }) +#define NM_DEVICE_ETHERNET_GET_PRIVATE(self) _NM_GET_PRIVATE_PTR(self, NMDeviceEthernet, NM_IS_DEVICE_ETHERNET) /*****************************************************************************/ diff --git a/src/devices/nm-device-veth.c b/src/devices/nm-device-veth.c index cca86fb..5692331 100644 --- a/src/devices/nm-device-veth.c +++ b/src/devices/nm-device-veth.c @@ -62,18 +62,7 @@ NM_GOBJECT_PROPERTIES_DEFINE (NMDeviceVeth, G_DEFINE_TYPE (NMDeviceVeth, nm_device_veth, NM_TYPE_DEVICE_ETHERNET) -#define NM_DEVICE_VETH_GET_PRIVATE(self) \ - ({ \ - /* preserve the const-ness of self. Unfortunately, that - * way, @self cannot be a void pointer */ \ - typeof (self) _self = (self); \ - \ - /* Get compiler error if variable is of wrong type */ \ - _nm_unused const NMDeviceVeth *_self2 = (_self); \ - \ - nm_assert (NM_IS_DEVICE_VETH (_self)); \ - &_self->_priv; \ - }) +#define NM_DEVICE_VETH_GET_PRIVATE(self) _NM_GET_PRIVATE(self, NMDeviceVeth, NM_IS_DEVICE_VETH) /*****************************************************************************/ diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 6939332..674563f 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -75,18 +75,7 @@ _LOG_DECLARE_SELF (NMDevice); G_DEFINE_ABSTRACT_TYPE (NMDevice, nm_device, NM_TYPE_EXPORTED_OBJECT) -#define NM_DEVICE_GET_PRIVATE(self) \ - ({ \ - /* preserve the const-ness of self. Unfortunately, that - * way, @self cannot be a void pointer */ \ - typeof (self) _self = (self); \ - \ - /* Get compiler error if variable is of wrong type */ \ - _nm_unused const NMDevice *_self2 = (_self); \ - \ - nm_assert (NM_IS_DEVICE (_self)); \ - _self->priv; \ - }) +#define NM_DEVICE_GET_PRIVATE(self) _NM_GET_PRIVATE_PTR(self, NMDevice, NM_IS_DEVICE) enum { STATE_CHANGED, @@ -2029,7 +2018,7 @@ link_type_compatible (NMDevice *self, return FALSE; } - device_type = self->priv->link_type; + device_type = self->_priv->link_type; if (device_type > NM_LINK_TYPE_UNKNOWN && device_type != link_type) { g_set_error (error, NM_DEVICE_ERROR, NM_DEVICE_ERROR_FAILED, "Needed link type 0x%x does not match the platform link type 0x%X", @@ -10058,7 +10047,7 @@ nm_device_set_unmanaged_by_user_udev (NMDevice *self) int ifindex; gboolean platform_unmanaged = FALSE; - ifindex = self->priv->ifindex; + ifindex = self->_priv->ifindex; if ( ifindex <= 0 || !nm_platform_link_get_unmanaged (NM_PLATFORM_GET, ifindex, &platform_unmanaged)) @@ -10145,7 +10134,7 @@ nm_device_reapply_settings_immediately (NMDevice *self) if (g_strcmp0 ((zone = nm_setting_connection_get_zone (s_con_settings)), nm_setting_connection_get_zone (s_con_applied)) != 0) { - version_id = nm_active_connection_version_id_bump ((NMActiveConnection *) self->priv->act_request); + version_id = nm_active_connection_version_id_bump ((NMActiveConnection *) self->_priv->act_request); _LOGD (LOGD_DEVICE, "reapply setting: zone = %s%s%s (version-id %llu)", NM_PRINT_FMT_QUOTE_STRING (zone), (long long unsigned) version_id); g_object_set (G_OBJECT (s_con_applied), @@ -10157,7 +10146,7 @@ nm_device_reapply_settings_immediately (NMDevice *self) if ((metered = nm_setting_connection_get_metered (s_con_settings)) != nm_setting_connection_get_metered (s_con_applied)) { - version_id = nm_active_connection_version_id_bump ((NMActiveConnection *) self->priv->act_request); + version_id = nm_active_connection_version_id_bump ((NMActiveConnection *) self->_priv->act_request); _LOGD (LOGD_DEVICE, "reapply setting: metered = %d (version-id %llu)", (int) metered, (long long unsigned) version_id); g_object_set (G_OBJECT (s_con_applied), @@ -10336,22 +10325,22 @@ nm_device_check_connection_available (NMDevice *self, static gboolean available_connections_del_all (NMDevice *self) { - if (g_hash_table_size (self->priv->available_connections) == 0) + if (g_hash_table_size (self->_priv->available_connections) == 0) return FALSE; - g_hash_table_remove_all (self->priv->available_connections); + g_hash_table_remove_all (self->_priv->available_connections); return TRUE; } static gboolean available_connections_add (NMDevice *self, NMConnection *connection) { - return nm_g_hash_table_add (self->priv->available_connections, g_object_ref (connection)); + return nm_g_hash_table_add (self->_priv->available_connections, g_object_ref (connection)); } static gboolean available_connections_del (NMDevice *self, NMConnection *connection) { - return g_hash_table_remove (self->priv->available_connections, connection); + return g_hash_table_remove (self->_priv->available_connections, connection); } static gboolean @@ -12097,7 +12086,7 @@ nm_device_init (NMDevice *self) priv = G_TYPE_INSTANCE_GET_PRIVATE (self, NM_TYPE_DEVICE, NMDevicePrivate); - self->priv = priv; + self->_priv = priv; priv->type = NM_DEVICE_TYPE_UNKNOWN; priv->capabilities = NM_DEVICE_CAP_NM_SUPPORTED; diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h index 34d31ca..be12ce7 100644 --- a/src/devices/nm-device.h +++ b/src/devices/nm-device.h @@ -125,7 +125,7 @@ struct _NMDevice { NMExportedObject parent; /* private */ - struct _NMDevicePrivate *priv; + struct _NMDevicePrivate *_priv; }; /* The flags have an relaxing meaning, that means, specifying more flags, can make diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 49c380a..f97e668 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -138,18 +138,7 @@ struct _NMDeviceWifiClass G_DEFINE_TYPE (NMDeviceWifi, nm_device_wifi, NM_TYPE_DEVICE) -#define NM_DEVICE_WIFI_GET_PRIVATE(self) \ - ({ \ - /* preserve the const-ness of self. Unfortunately, that - * way, @self cannot be a void pointer */ \ - typeof (self) _self = (self); \ - \ - /* Get compiler error if variable is of wrong type */ \ - _nm_unused const NMDeviceWifi *_self2 = (_self); \ - \ - nm_assert (NM_IS_DEVICE_WIFI (_self)); \ - &_self->_priv; \ - }) +#define NM_DEVICE_WIFI_GET_PRIVATE(self) _NM_GET_PRIVATE(self, NMDeviceWifi, NM_IS_DEVICE_WIFI) /*****************************************************************************/ diff --git a/src/devices/wifi/nm-wifi-ap.c b/src/devices/wifi/nm-wifi-ap.c index e1beb85..2232f82 100644 --- a/src/devices/wifi/nm-wifi-ap.c +++ b/src/devices/wifi/nm-wifi-ap.c @@ -68,18 +68,7 @@ struct _NMAccessPointClass{ NMExportedObjectClass parent; }; -#define NM_AP_GET_PRIVATE(self) \ - ({ \ - /* preserve the const-ness of self. Unfortunately, that - * way, @self cannot be a void pointer */ \ - typeof (self) _self = (self); \ - \ - /* Get compiler error if variable is of wrong type */ \ - _nm_unused const NMAccessPoint *_self2 = (_self); \ - \ - nm_assert (NM_IS_AP (_self)); \ - &_self->_priv; \ - }) +#define NM_AP_GET_PRIVATE(self) _NM_GET_PRIVATE(self, NMAccessPoint, NM_IS_AP) G_DEFINE_TYPE (NMAccessPoint, nm_ap, NM_TYPE_EXPORTED_OBJECT) diff --git a/src/dns-manager/nm-dns-manager.c b/src/dns-manager/nm-dns-manager.c index 3efd5ac..ce8f4d9 100644 --- a/src/dns-manager/nm-dns-manager.c +++ b/src/dns-manager/nm-dns-manager.c @@ -151,18 +151,7 @@ G_DEFINE_TYPE (NMDnsManager, nm_dns_manager, G_TYPE_OBJECT) NM_DEFINE_SINGLETON_INSTANCE (NMDnsManager); -#define NM_DNS_MANAGER_GET_PRIVATE(self) \ - ({ \ - /* preserve the const-ness of self. Unfortunately, that - * way, @self cannot be a void pointer */ \ - typeof (self) _self = (self); \ - \ - /* Get compiler error if variable is of wrong type */ \ - _nm_unused const NMDnsManager *_self2 = (_self); \ - \ - nm_assert (NM_IS_DNS_MANAGER (_self)); \ - &_self->_priv; \ - }) +#define NM_DNS_MANAGER_GET_PRIVATE(self) _NM_GET_PRIVATE(self, NMDnsManager, NM_IS_DNS_MANAGER) /*****************************************************************************/ diff --git a/src/nm-auth-subject.c b/src/nm-auth-subject.c index eb496b2..54d3595 100644 --- a/src/nm-auth-subject.c +++ b/src/nm-auth-subject.c @@ -68,18 +68,7 @@ struct _NMAuthSubjectClass { G_DEFINE_TYPE (NMAuthSubject, nm_auth_subject, G_TYPE_OBJECT) -#define NM_AUTH_SUBJECT_GET_PRIVATE(self) \ - ({ \ - /* preserve the const-ness of self. Unfortunately, that - * way, @self cannot be a void pointer */ \ - typeof (self) _self = (self); \ - \ - /* Get compiler error if variable is of wrong type */ \ - _nm_unused const NMAuthSubject *_self2 = (_self); \ - \ - nm_assert (NM_IS_AUTH_SUBJECT (_self)); \ - &_self->_priv; \ - }) +#define NM_AUTH_SUBJECT_GET_PRIVATE(self) _NM_GET_PRIVATE(self, NMAuthSubject, NM_IS_AUTH_SUBJECT) /**************************************************************/ diff --git a/src/nm-checkpoint.c b/src/nm-checkpoint.c index cb1adc3..605e070 100644 --- a/src/nm-checkpoint.c +++ b/src/nm-checkpoint.c @@ -70,7 +70,7 @@ typedef struct { struct _NMCheckpoint { NMExportedObject parent; - NMCheckpointPrivate priv; + NMCheckpointPrivate _priv; }; typedef struct { @@ -79,18 +79,7 @@ typedef struct { G_DEFINE_TYPE (NMCheckpoint, nm_checkpoint, NM_TYPE_EXPORTED_OBJECT) -#define NM_CHECKPOINT_GET_PRIVATE(self) \ - ({ \ - /* preserve the const-ness of self. Unfortunately, that - * way, @self cannot be a void pointer */ \ - typeof (self) _self = (self); \ - \ - /* Get compiler error if variable is of wrong type */ \ - _nm_unused const NMCheckpoint *_self2 = (_self); \ - \ - nm_assert (NM_IS_CHECKPOINT (_self)); \ - &_self->priv; \ - }) +#define NM_CHECKPOINT_GET_PRIVATE(self) _NM_GET_PRIVATE(self, NMCheckpoint, NM_IS_CHECKPOINT) NM_GOBJECT_PROPERTIES_DEFINE_BASE ( PROP_DEVICES, diff --git a/src/nm-ip4-config.c b/src/nm-ip4-config.c index 22d1d07..a4d4361 100644 --- a/src/nm-ip4-config.c +++ b/src/nm-ip4-config.c @@ -68,18 +68,7 @@ struct _NMIP4ConfigClass { G_DEFINE_TYPE (NMIP4Config, nm_ip4_config, NM_TYPE_EXPORTED_OBJECT) -#define NM_IP4_CONFIG_GET_PRIVATE(self) \ - ({ \ - /* preserve the const-ness of self. Unfortunately, that - * way, @self cannot be a void pointer */ \ - typeof (self) _self = (self); \ - \ - /* Get compiler error if variable is of wrong type */ \ - _nm_unused const NMIP4Config *_self2 = (_self); \ - \ - nm_assert (NM_IS_IP4_CONFIG (_self)); \ - &_self->_priv; \ - }) +#define NM_IP4_CONFIG_GET_PRIVATE(self) _NM_GET_PRIVATE(self, NMIP4Config, NM_IS_IP4_CONFIG) /* internal guint32 are assigned to gobject properties of type uint. Ensure, that uint is large enough */ G_STATIC_ASSERT (sizeof (uint) >= sizeof (guint32)); diff --git a/src/nm-ip6-config.c b/src/nm-ip6-config.c index ac9e6cd..8002d61 100644 --- a/src/nm-ip6-config.c +++ b/src/nm-ip6-config.c @@ -61,18 +61,7 @@ struct _NMIP6ConfigClass { G_DEFINE_TYPE (NMIP6Config, nm_ip6_config, NM_TYPE_EXPORTED_OBJECT) -#define NM_IP6_CONFIG_GET_PRIVATE(self) \ - ({ \ - /* preserve the const-ness of self. Unfortunately, that - * way, @self cannot be a void pointer */ \ - typeof (self) _self = (self); \ - \ - /* Get compiler error if variable is of wrong type */ \ - _nm_unused const NMIP6Config *_self2 = (_self); \ - \ - nm_assert (NM_IS_IP6_CONFIG (_self)); \ - &_self->_priv; \ - }) +#define NM_IP6_CONFIG_GET_PRIVATE(self) _NM_GET_PRIVATE(self, NMIP6Config, NM_IS_IP6_CONFIG) NM_GOBJECT_PROPERTIES_DEFINE (NMIP6Config, PROP_IFINDEX, diff --git a/src/nm-manager.c b/src/nm-manager.c index 5794bb9..9ad6517 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -158,21 +158,10 @@ typedef struct { NMExportedObjectClass parent; } NMManagerClass; -#define NM_MANAGER_GET_PRIVATE(self) \ - ({ \ - /* preserve the const-ness of self. Unfortunately, that - * way, @self cannot be a void pointer */ \ - typeof (self) _self = (self); \ - \ - /* Get compiler error if variable is of wrong type */ \ - _nm_unused const NMManager *_self2 = (_self); \ - \ - nm_assert (NM_IS_MANAGER (_self)); \ - &_self->_priv; \ - }) - G_DEFINE_TYPE (NMManager, nm_manager, NM_TYPE_EXPORTED_OBJECT) +#define NM_MANAGER_GET_PRIVATE(self) _NM_GET_PRIVATE(self, NMManager, NM_IS_MANAGER) + enum { DEVICE_ADDED, INTERNAL_DEVICE_ADDED, diff --git a/src/rdisc/nm-lndp-rdisc.c b/src/rdisc/nm-lndp-rdisc.c index 12c2e30..db2965b 100644 --- a/src/rdisc/nm-lndp-rdisc.c +++ b/src/rdisc/nm-lndp-rdisc.c @@ -60,18 +60,7 @@ struct _NMLndpRDiscClass { G_DEFINE_TYPE (NMLndpRDisc, nm_lndp_rdisc, NM_TYPE_RDISC) -#define NM_LNDP_RDISC_GET_PRIVATE(self) \ - ({ \ - /* preserve the const-ness of self. Unfortunately, that - * way, @self cannot be a void pointer */ \ - typeof (self) _self = (self); \ - \ - /* Get compiler error if variable is of wrong type */ \ - _nm_unused const NMLndpRDisc *_self2 = (_self); \ - \ - nm_assert (NM_IS_LNDP_RDISC (_self)); \ - &_self->_priv; \ - }) +#define NM_LNDP_RDISC_GET_PRIVATE(self) _NM_GET_PRIVATE(self, NMLndpRDisc, NM_IS_LNDP_RDISC) /*****************************************************************************/ diff --git a/src/rdisc/nm-rdisc.c b/src/rdisc/nm-rdisc.c index cf993bc..3bc6975 100644 --- a/src/rdisc/nm-rdisc.c +++ b/src/rdisc/nm-rdisc.c @@ -87,18 +87,7 @@ static guint signals[LAST_SIGNAL] = { 0 }; G_DEFINE_TYPE (NMRDisc, nm_rdisc, G_TYPE_OBJECT) -#define NM_RDISC_GET_PRIVATE(self) \ - ({ \ - /* preserve the const-ness of self. Unfortunately, that - * way, @self cannot be a void pointer */ \ - typeof (self) _self = (self); \ - \ - /* Get compiler error if variable is of wrong type */ \ - _nm_unused const NMRDisc *_self2 = (_self); \ - \ - nm_assert (NM_IS_RDISC (_self)); \ - _self->_priv; \ - }) +#define NM_RDISC_GET_PRIVATE(self) _NM_GET_PRIVATE_PTR(self, NMRDisc, NM_IS_RDISC) /*****************************************************************************/ diff --git a/src/vpn-manager/nm-vpn-connection.c b/src/vpn-manager/nm-vpn-connection.c index 92c5bd8..69b45dc 100644 --- a/src/vpn-manager/nm-vpn-connection.c +++ b/src/vpn-manager/nm-vpn-connection.c @@ -168,18 +168,7 @@ struct _NMVpnConnectionClass { G_DEFINE_TYPE (NMVpnConnection, nm_vpn_connection, NM_TYPE_ACTIVE_CONNECTION) -#define NM_VPN_CONNECTION_GET_PRIVATE(self) \ - ({ \ - /* preserve the const-ness of self. Unfortunately, that - * way, @self cannot be a void pointer */ \ - typeof (self) _self = (self); \ - \ - /* Get compiler error if variable is of wrong type */ \ - _nm_unused const NMVpnConnection *_self2 = (_self); \ - \ - nm_assert (NM_IS_VPN_CONNECTION (_self)); \ - &_self->_priv; \ - }) +#define NM_VPN_CONNECTION_GET_PRIVATE(self) _NM_GET_PRIVATE(self, NMVpnConnection, NM_IS_VPN_CONNECTION) /*****************************************************************************/ -- 2.7.4 From f25940f2518c3789a5f18d1b458e53a95dc88af7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 5 Sep 2016 16:42:59 +0200 Subject: [PATCH 05/11] dhcp-listener: refactor type definition and embed private data in @self (cherry picked from commit 822f01a8fdb63831c887d5db9fb06eb840f53c88) (cherry picked from commit 75e13f0e15b7618d7fafbe3f1b990871be0950a7) --- src/dhcp-manager/nm-dhcp-listener.c | 50 +++++++++++++++++++++---------------- src/dhcp-manager/nm-dhcp-listener.h | 4 +-- 2 files changed, 31 insertions(+), 23 deletions(-) diff --git a/src/dhcp-manager/nm-dhcp-listener.c b/src/dhcp-manager/nm-dhcp-listener.c index eadff3e..d3616cb 100644 --- a/src/dhcp-manager/nm-dhcp-listener.c +++ b/src/dhcp-manager/nm-dhcp-listener.c @@ -13,12 +13,14 @@ * with this program; if not, write to the Free Software Foundation, Inc., * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. * - * Copyright 2014 Red Hat, Inc. + * Copyright 2014 - 2016 Red Hat, Inc. * */ #include "nm-default.h" +#include "nm-dhcp-listener.h" + #include #include #include @@ -27,7 +29,6 @@ #include #include -#include "nm-dhcp-listener.h" #include "nm-core-internal.h" #include "nm-bus-manager.h" #include "NetworkManagerUtils.h" @@ -36,6 +37,8 @@ #define PRIV_SOCK_PATH NMRUNDIR "/private-dhcp" #define PRIV_SOCK_TAG "dhcp" +/*****************************************************************************/ + typedef struct { NMBusManager * dbus_mgr; gulong new_conn_id; @@ -43,17 +46,26 @@ typedef struct { GHashTable * signal_handlers; } NMDhcpListenerPrivate; -#define NM_DHCP_LISTENER_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), NM_TYPE_DHCP_LISTENER, NMDhcpListenerPrivate)) +struct _NMDhcpListener { + GObject parent; + NMDhcpListenerPrivate _priv; +}; + +struct _NMDhcpListenerClass { + GObjectClass parent_class; +}; G_DEFINE_TYPE (NMDhcpListener, nm_dhcp_listener, G_TYPE_OBJECT) +#define NM_DHCP_LISTENER_GET_PRIVATE(self) _NM_GET_PRIVATE(self, NMDhcpListener, NM_IS_DHCP_LISTENER) + enum { EVENT, LAST_SIGNAL }; static guint signals[LAST_SIGNAL] = { 0 }; -/***************************************************/ +/*****************************************************************************/ static char * get_option (GVariant *options, const char *key) @@ -188,7 +200,7 @@ nm_dhcp_listener_init (NMDhcpListener *self) { NMDhcpListenerPrivate *priv = NM_DHCP_LISTENER_GET_PRIVATE (self); - /* Maps GDBusConnection :: GDBusProxy */ + /* Maps GDBusConnection :: signal-id */ priv->signal_handlers = g_hash_table_new (NULL, NULL); priv->dbus_mgr = nm_bus_manager_get (); @@ -208,7 +220,7 @@ nm_dhcp_listener_init (NMDhcpListener *self) static void dispose (GObject *object) { - NMDhcpListenerPrivate *priv = NM_DHCP_LISTENER_GET_PRIVATE (object); + NMDhcpListenerPrivate *priv = NM_DHCP_LISTENER_GET_PRIVATE ((NMDhcpListener *) object); nm_clear_g_signal_handler (priv->dbus_mgr, &priv->new_conn_id); nm_clear_g_signal_handler (priv->dbus_mgr, &priv->dis_conn_id); @@ -224,22 +236,18 @@ nm_dhcp_listener_class_init (NMDhcpListenerClass *listener_class) { GObjectClass *object_class = G_OBJECT_CLASS (listener_class); - g_type_class_add_private (listener_class, sizeof (NMDhcpListenerPrivate)); - - /* virtual methods */ object_class->dispose = dispose; - /* signals */ signals[EVENT] = - g_signal_new (NM_DHCP_LISTENER_EVENT, - G_OBJECT_CLASS_TYPE (object_class), - G_SIGNAL_RUN_LAST, 0, - g_signal_accumulator_true_handled, - NULL, NULL, - G_TYPE_BOOLEAN, /* listeners return TRUE if handled */ - 4, - G_TYPE_STRING, /* iface */ - G_TYPE_INT, /* pid */ - G_TYPE_VARIANT, /* options */ - G_TYPE_STRING); /* reason */ + g_signal_new (NM_DHCP_LISTENER_EVENT, + G_OBJECT_CLASS_TYPE (object_class), + G_SIGNAL_RUN_LAST, 0, + g_signal_accumulator_true_handled, + NULL, NULL, + G_TYPE_BOOLEAN, /* listeners return TRUE if handled */ + 4, + G_TYPE_STRING, /* iface */ + G_TYPE_INT, /* pid */ + G_TYPE_VARIANT, /* options */ + G_TYPE_STRING); /* reason */ } diff --git a/src/dhcp-manager/nm-dhcp-listener.h b/src/dhcp-manager/nm-dhcp-listener.h index ff31fe3..3018b97 100644 --- a/src/dhcp-manager/nm-dhcp-listener.h +++ b/src/dhcp-manager/nm-dhcp-listener.h @@ -26,8 +26,8 @@ #define NM_DHCP_LISTENER_EVENT "event" -typedef GObject NMDhcpListener; -typedef GObjectClass NMDhcpListenerClass; +typedef struct _NMDhcpListener NMDhcpListener; +typedef struct _NMDhcpListenerClass NMDhcpListenerClass; GType nm_dhcp_listener_get_type (void); -- 2.7.4 From 6d59141bd7459320a63ba606587d757cbd0aa74d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 5 Sep 2016 17:17:09 +0200 Subject: [PATCH 06/11] dhcp-listener: add logging macros to nm-dhcp-listener.c (cherry picked from commit d37cd04fe059e9d99c7103f4e22a9a945f8d4d98) (cherry picked from commit 3920a90e4a63b69bda583c28cc6bdfef1a9f0470) --- src/dhcp-manager/nm-dhcp-listener.c | 39 ++++++++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/src/dhcp-manager/nm-dhcp-listener.c b/src/dhcp-manager/nm-dhcp-listener.c index d3616cb..79d3513 100644 --- a/src/dhcp-manager/nm-dhcp-listener.c +++ b/src/dhcp-manager/nm-dhcp-listener.c @@ -55,16 +55,35 @@ struct _NMDhcpListenerClass { GObjectClass parent_class; }; -G_DEFINE_TYPE (NMDhcpListener, nm_dhcp_listener, G_TYPE_OBJECT) - -#define NM_DHCP_LISTENER_GET_PRIVATE(self) _NM_GET_PRIVATE(self, NMDhcpListener, NM_IS_DHCP_LISTENER) - enum { EVENT, LAST_SIGNAL }; static guint signals[LAST_SIGNAL] = { 0 }; +G_DEFINE_TYPE (NMDhcpListener, nm_dhcp_listener, G_TYPE_OBJECT) + +#define NM_DHCP_LISTENER_GET_PRIVATE(self) _NM_GET_PRIVATE(self, NMDhcpListener, NM_IS_DHCP_LISTENER) + +NM_DEFINE_SINGLETON_GETTER (NMDhcpListener, nm_dhcp_listener_get, NM_TYPE_DHCP_LISTENER); + +/*****************************************************************************/ + +#define _NMLOG_PREFIX_NAME "dhcp-listener" +#define _NMLOG_DOMAIN LOGD_DHCP +#define _NMLOG(level, ...) \ + G_STMT_START { \ + const NMDhcpListener *_self = (self); \ + char _prefix[64]; \ + \ + nm_log ((level), (_NMLOG_DOMAIN), \ + "%s: " _NM_UTILS_MACRO_FIRST(__VA_ARGS__), \ + (_self != singleton_instance \ + ? nm_sprintf_buf (_prefix, "%s[%p]", _NMLOG_PREFIX_NAME, _self) \ + : _NMLOG_PREFIX_NAME )\ + _NM_UTILS_MACRO_REST(__VA_ARGS__)); \ + } G_STMT_END + /*****************************************************************************/ static char * @@ -123,20 +142,20 @@ handle_event (GDBusConnection *connection, iface = get_option (options, "interface"); if (iface == NULL) { - nm_log_warn (LOGD_DHCP, "dhcp-event: didn't have associated interface."); + _LOGW ("dhcp-event: didn't have associated interface."); goto out; } pid_str = get_option (options, "pid"); pid = _nm_utils_ascii_str_to_int64 (pid_str, 10, 0, G_MAXINT32, -1); if (pid == -1) { - nm_log_warn (LOGD_DHCP, "dhcp-event: couldn't convert PID '%s' to an integer", pid_str ? pid_str : "(null)"); + _LOGW ("dhcp-event: couldn't convert PID '%s' to an integer", pid_str ? pid_str : "(null)"); goto out; } reason = get_option (options, "reason"); if (reason == NULL) { - nm_log_warn (LOGD_DHCP, "dhcp-event: (pid %d) DHCP event didn't have a reason", pid); + _LOGW ("dhcp-event: (pid %d) DHCP event didn't have a reason", pid); goto out; } @@ -144,9 +163,9 @@ handle_event (GDBusConnection *connection, if (!handled) { if (g_ascii_strcasecmp (reason, "RELEASE") == 0) { /* Ignore event when the dhcp client gets killed and we receive its last message */ - nm_log_dbg (LOGD_DHCP, "dhcp-event: (pid %d) unhandled RELEASE DHCP event for interface %s", pid, iface); + _LOGD ("dhcp-event: (pid %d) unhandled RELEASE DHCP event for interface %s", pid, iface); } else - nm_log_warn (LOGD_DHCP, "dhcp-event: (pid %d) unhandled DHCP event for interface %s", pid, iface); + _LOGW ("dhcp-event: (pid %d) unhandled DHCP event for interface %s", pid, iface); } out: @@ -193,8 +212,6 @@ dis_connection_cb (NMBusManager *mgr, /***************************************************/ -NM_DEFINE_SINGLETON_GETTER (NMDhcpListener, nm_dhcp_listener_get, NM_TYPE_DHCP_LISTENER); - static void nm_dhcp_listener_init (NMDhcpListener *self) { -- 2.7.4 From 5ac925ed8c718aa9ec0ab405680ed858fd79f8a6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 5 Sep 2016 17:24:28 +0200 Subject: [PATCH 07/11] dhcp-listener/trivial: rename field to track connections in NMDhcpListener It's not "signal-handles", as it currently tracks the registration ID of type int. Rename it, it is effectively the list of connections that we track. (cherry picked from commit 2dd3a5245f91a7c25ae1a36a86638195500f00a8) (cherry picked from commit 0ebdfd6cf1f1ba3c9753db4d9dd8979bb458a689) --- src/dhcp-manager/nm-dhcp-listener.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/dhcp-manager/nm-dhcp-listener.c b/src/dhcp-manager/nm-dhcp-listener.c index 79d3513..b09bc20 100644 --- a/src/dhcp-manager/nm-dhcp-listener.c +++ b/src/dhcp-manager/nm-dhcp-listener.c @@ -43,7 +43,7 @@ typedef struct { NMBusManager * dbus_mgr; gulong new_conn_id; gulong dis_conn_id; - GHashTable * signal_handlers; + GHashTable * connections; } NMDhcpListenerPrivate; struct _NMDhcpListener { @@ -192,7 +192,7 @@ new_connection_cb (NMBusManager *mgr, NULL, G_DBUS_SIGNAL_FLAGS_NONE, handle_event, self, NULL); - g_hash_table_insert (priv->signal_handlers, connection, GUINT_TO_POINTER (id)); + g_hash_table_insert (priv->connections, connection, GUINT_TO_POINTER (id)); } static void @@ -203,10 +203,10 @@ dis_connection_cb (NMBusManager *mgr, NMDhcpListenerPrivate *priv = NM_DHCP_LISTENER_GET_PRIVATE (self); guint id; - id = GPOINTER_TO_UINT (g_hash_table_lookup (priv->signal_handlers, connection)); + id = GPOINTER_TO_UINT (g_hash_table_lookup (priv->connections, connection)); if (id) { g_dbus_connection_signal_unsubscribe (connection, id); - g_hash_table_remove (priv->signal_handlers, connection); + g_hash_table_remove (priv->connections, connection); } } @@ -218,7 +218,7 @@ nm_dhcp_listener_init (NMDhcpListener *self) NMDhcpListenerPrivate *priv = NM_DHCP_LISTENER_GET_PRIVATE (self); /* Maps GDBusConnection :: signal-id */ - priv->signal_handlers = g_hash_table_new (NULL, NULL); + priv->connections = g_hash_table_new (NULL, NULL); priv->dbus_mgr = nm_bus_manager_get (); @@ -243,7 +243,7 @@ dispose (GObject *object) nm_clear_g_signal_handler (priv->dbus_mgr, &priv->dis_conn_id); priv->dbus_mgr = NULL; - g_clear_pointer (&priv->signal_handlers, g_hash_table_destroy); + g_clear_pointer (&priv->connections, g_hash_table_destroy); G_OBJECT_CLASS (nm_dhcp_listener_parent_class)->dispose (object); } -- 2.7.4 From 35518c37de240061c26364b98e464ba140dcdc69 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 5 Sep 2016 11:06:04 +0200 Subject: [PATCH 08/11] dhcp-helper: refactor error handling Don't exit(1) from fatal_error() because that skips destroying local variables in main(). Just return regularly. (cherry picked from commit bb489163db36889a6fb80789e4e5b9dd8a15dbdd) (cherry picked from commit a8d87ef87f2931f07e6fa6dd0df56bf3d1fdf79e) --- src/dhcp-manager/nm-dhcp-helper.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/dhcp-manager/nm-dhcp-helper.c b/src/dhcp-manager/nm-dhcp-helper.c index 7667084..8c512e7 100644 --- a/src/dhcp-manager/nm-dhcp-helper.c +++ b/src/dhcp-manager/nm-dhcp-helper.c @@ -74,26 +74,26 @@ build_signal_parameters (void) } static void -fatal_error (void) +kill_pid (void) { - const char *pid_str = getenv ("pid"); - int pid = 0; + const char *pid_str; + pid_t pid = 0; + pid_str = getenv ("pid"); if (pid_str) pid = strtol (pid_str, NULL, 10); if (pid) { g_printerr ("Fatal error occured, killing dhclient instance with pid %d.\n", pid); kill (pid, SIGTERM); } - - exit (1); } int main (int argc, char *argv[]) { - GDBusConnection *connection; - GError *error = NULL; + gs_unref_object GDBusConnection *connection = NULL; + gs_free_error GError *error = NULL; + gboolean success = FALSE; nm_g_type_init (); @@ -104,8 +104,7 @@ main (int argc, char *argv[]) g_dbus_error_strip_remote_error (error); g_printerr ("Error: could not connect to NetworkManager D-Bus socket: %s\n", error->message); - g_error_free (error); - fatal_error (); + goto out; } if (!g_dbus_connection_emit_signal (connection, @@ -117,18 +116,19 @@ main (int argc, char *argv[]) &error)) { g_dbus_error_strip_remote_error (error); g_printerr ("Error: Could not send DHCP Event signal: %s\n", error->message); - g_error_free (error); - fatal_error (); + goto out; } if (!g_dbus_connection_flush_sync (connection, NULL, &error)) { g_dbus_error_strip_remote_error (error); g_printerr ("Error: Could not flush D-Bus connection: %s\n", error->message); - g_error_free (error); - fatal_error (); + goto out; } - g_object_unref (connection); - return 0; + success = TRUE; +out: + if (!success) + kill_pid (); + return success ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.7.4 From e342765675214ef1fbcdd9e30d916c687a75fd6e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 5 Sep 2016 13:38:58 +0200 Subject: [PATCH 09/11] dhcp-helper: refactor logging to use logging macros (cherry picked from commit cc89996c9e826c9c8d12d5fb7bc8a2a578209eb0) (cherry picked from commit 9d44dafc3cef8fe1b7d41c9af0f6fa30254924f1) --- src/dhcp-manager/nm-dhcp-helper.c | 45 ++++++++++++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/src/dhcp-manager/nm-dhcp-helper.c b/src/dhcp-manager/nm-dhcp-helper.c index 8c512e7..383c985 100644 --- a/src/dhcp-manager/nm-dhcp-helper.c +++ b/src/dhcp-manager/nm-dhcp-helper.c @@ -25,8 +25,43 @@ #include #include +#include "nm-utils/nm-vpn-plugin-macros.h" + #define NM_DHCP_CLIENT_DBUS_IFACE "org.freedesktop.nm_dhcp_client" +/*****************************************************************************/ + +#ifdef NM_MORE_LOGGING +#define _NMLOG_ENABLED(level) TRUE +#else +#define _NMLOG_ENABLED(level) ((level) <= LOG_ERR) +#endif + +#define _NMLOG(always_enabled, level, ...) \ + G_STMT_START { \ + if ((always_enabled) || _NMLOG_ENABLED (level)) { \ + GTimeVal _tv; \ + \ + g_get_current_time (&_tv); \ + g_print ("nm-dhcp-helper[%ld] %-7s [%ld.%04ld] " _NM_UTILS_MACRO_FIRST (__VA_ARGS__) "\n", \ + (long) getpid (), \ + nm_utils_syslog_to_str (level), \ + _tv.tv_sec, _tv.tv_usec / 100 \ + _NM_UTILS_MACRO_REST (__VA_ARGS__)); \ + } \ + } G_STMT_END + +#define _LOGD(...) _NMLOG(TRUE, LOG_INFO, __VA_ARGS__) +#define _LOGI(...) _NMLOG(TRUE, LOG_NOTICE, __VA_ARGS__) +#define _LOGW(...) _NMLOG(TRUE, LOG_WARNING, __VA_ARGS__) +#define _LOGE(...) _NMLOG(TRUE, LOG_ERR, __VA_ARGS__) + +#define _LOGd(...) _NMLOG(FALSE, LOG_INFO, __VA_ARGS__) +#define _LOGi(...) _NMLOG(FALSE, LOG_NOTICE, __VA_ARGS__) +#define _LOGw(...) _NMLOG(FALSE, LOG_WARNING, __VA_ARGS__) + +/*****************************************************************************/ + static const char * ignore[] = {"PATH", "SHLVL", "_", "PWD", "dhc_dbus", NULL}; static GVariant * @@ -83,7 +118,7 @@ kill_pid (void) if (pid_str) pid = strtol (pid_str, NULL, 10); if (pid) { - g_printerr ("Fatal error occured, killing dhclient instance with pid %d.\n", pid); + _LOGI ("a fatal error occured, kill dhclient instance with pid %d\n", pid); kill (pid, SIGTERM); } } @@ -102,8 +137,8 @@ main (int argc, char *argv[]) NULL, NULL, &error); if (!connection) { g_dbus_error_strip_remote_error (error); - g_printerr ("Error: could not connect to NetworkManager D-Bus socket: %s\n", - error->message); + _LOGE ("could not connect to NetworkManager D-Bus socket: %s", + error->message); goto out; } @@ -115,13 +150,13 @@ main (int argc, char *argv[]) build_signal_parameters (), &error)) { g_dbus_error_strip_remote_error (error); - g_printerr ("Error: Could not send DHCP Event signal: %s\n", error->message); + _LOGE ("could not send DHCP Event signal: %s", error->message); goto out; } if (!g_dbus_connection_flush_sync (connection, NULL, &error)) { g_dbus_error_strip_remote_error (error); - g_printerr ("Error: Could not flush D-Bus connection: %s\n", error->message); + _LOGE ("could not flush D-Bus connection: %s", error->message); goto out; } -- 2.7.4 From 8fb42bd50ad41167498332e48bea548cdf76453c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 5 Sep 2016 11:54:46 +0200 Subject: [PATCH 10/11] dhcp: add new header "nm-dhcp-helper-api.h" (cherry picked from commit 7684b68c49812ed7b2ec493889fae04db066b665) (cherry picked from commit 3ac3125aff9987b3ac0284daaa3faae93291a603) --- src/Makefile.am | 1 + src/dhcp-manager/Makefile.am | 5 ++++- src/dhcp-manager/nm-dhcp-helper-api.h | 31 +++++++++++++++++++++++++++++++ src/dhcp-manager/nm-dhcp-helper.c | 2 +- src/dhcp-manager/nm-dhcp-listener.c | 2 +- 5 files changed, 38 insertions(+), 3 deletions(-) create mode 100644 src/dhcp-manager/nm-dhcp-helper-api.h diff --git a/src/Makefile.am b/src/Makefile.am index c460caf..8d29b19 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -335,6 +335,7 @@ libNetworkManager_la_SOURCES = \ dhcp-manager/nm-dhcp-client.c \ dhcp-manager/nm-dhcp-client.h \ dhcp-manager/nm-dhcp-client-logging.h \ + dhcp-manager/nm-dhcp-helper-api.h \ dhcp-manager/nm-dhcp-utils.c \ dhcp-manager/nm-dhcp-utils.h \ dhcp-manager/nm-dhcp-listener.c \ diff --git a/src/dhcp-manager/Makefile.am b/src/dhcp-manager/Makefile.am index b4590b4..4295412 100644 --- a/src/dhcp-manager/Makefile.am +++ b/src/dhcp-manager/Makefile.am @@ -1,6 +1,9 @@ libexec_PROGRAMS = nm-dhcp-helper -nm_dhcp_helper_SOURCES = nm-dhcp-helper.c +nm_dhcp_helper_SOURCES = \ + nm-dhcp-helper.c \ + nm-dhcp-helper-api.h \ + $(NULL) nm_dhcp_helper_CPPFLAGS = \ $(GLIB_CFLAGS) \ diff --git a/src/dhcp-manager/nm-dhcp-helper-api.h b/src/dhcp-manager/nm-dhcp-helper-api.h new file mode 100644 index 0000000..a6323db --- /dev/null +++ b/src/dhcp-manager/nm-dhcp-helper-api.h @@ -0,0 +1,31 @@ +/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */ +/* NetworkManager -- Network link manager + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library 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 + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the + * Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, + * Boston, MA 02110-1301 USA. + * + * (C) Copyright 2016 Red Hat, Inc. + */ + +#ifndef __NM_DHCP_HELPER_API_H__ +#define __NM_DHCP_HELPER_API_H__ + +/******************************************************************************/ + +#define NM_DHCP_CLIENT_DBUS_IFACE "org.freedesktop.nm_dhcp_client" + +/******************************************************************************/ + +#endif /* __NM_DHCP_HELPER_API_H__ */ diff --git a/src/dhcp-manager/nm-dhcp-helper.c b/src/dhcp-manager/nm-dhcp-helper.c index 383c985..e53fe27 100644 --- a/src/dhcp-manager/nm-dhcp-helper.c +++ b/src/dhcp-manager/nm-dhcp-helper.c @@ -27,7 +27,7 @@ #include "nm-utils/nm-vpn-plugin-macros.h" -#define NM_DHCP_CLIENT_DBUS_IFACE "org.freedesktop.nm_dhcp_client" +#include "nm-dhcp-helper-api.h" /*****************************************************************************/ diff --git a/src/dhcp-manager/nm-dhcp-listener.c b/src/dhcp-manager/nm-dhcp-listener.c index b09bc20..7e9de49 100644 --- a/src/dhcp-manager/nm-dhcp-listener.c +++ b/src/dhcp-manager/nm-dhcp-listener.c @@ -29,11 +29,11 @@ #include #include +#include "nm-dhcp-helper-api.h" #include "nm-core-internal.h" #include "nm-bus-manager.h" #include "NetworkManagerUtils.h" -#define NM_DHCP_CLIENT_DBUS_IFACE "org.freedesktop.nm_dhcp_client" #define PRIV_SOCK_PATH NMRUNDIR "/private-dhcp" #define PRIV_SOCK_TAG "dhcp" -- 2.7.4 From 901ac5654ae51fd44b575c21201f41f5e6f2bbd8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 5 Sep 2016 12:32:40 +0200 Subject: [PATCH 11/11] dhcp: call synchronous Notify D-Bus method from nm-dhcp-helper A D-Bus signal is asynchronous and it can happen that nm-dhcp-helper emits the "Event" signal before the server is able to register a handler: NM_DHCP_HELPER=/usr/libexec/nm-dhcp-helper nmcli general logging level TRACE for i in `seq 1 500`; do $NM_DHCP_HELPER & done journalctl -u NetworkManager --since '1 min ago' | grep "didn't have associated interface" | wc -l 499 Avoid that, by calling the synchronous D-Bus method "Notify". Interestingly, this race seem to exist since 2007. Actually, we called g_dbus_connection_signal_subscribe() from inside GDBusServer:new-connection signal. So it is not clear how such a race could exist. I was not able to reproduce it by putting a sleep before g_dbus_connection_signal_subscribe(). On the other hand, there is bug rh#1372854 and above reproducer which strongly indicates that events can be lost under certain circumstances. Now we instead g_dbus_connection_register_object() from the new-connection signal. According to my tests there was no more race as also backed by glib's documentation. Still, keep a simple retry-loop in nm-dhcp-helper just to be sure. https://bugzilla.redhat.com/show_bug.cgi?id=1372854 https://bugzilla.redhat.com/show_bug.cgi?id=1373276 (cherry picked from commit 2856a658b38df88494187c811415a198a424e1f2) (cherry picked from commit e678bd29a4e5fdd83b7d99392d54ecb5fcabc287) --- src/dhcp-manager/nm-dhcp-helper-api.h | 5 ++ src/dhcp-manager/nm-dhcp-helper.c | 78 +++++++++++++++++++++++----- src/dhcp-manager/nm-dhcp-listener.c | 97 ++++++++++++++++++++++++++++------- src/nm-bus-manager.c | 6 ++- 4 files changed, 153 insertions(+), 33 deletions(-) diff --git a/src/dhcp-manager/nm-dhcp-helper-api.h b/src/dhcp-manager/nm-dhcp-helper-api.h index a6323db..a3eb171 100644 --- a/src/dhcp-manager/nm-dhcp-helper-api.h +++ b/src/dhcp-manager/nm-dhcp-helper-api.h @@ -26,6 +26,11 @@ #define NM_DHCP_CLIENT_DBUS_IFACE "org.freedesktop.nm_dhcp_client" +#define NM_DHCP_HELPER_SERVER_BUS_NAME "org.freedesktop.nm_dhcp_server" +#define NM_DHCP_HELPER_SERVER_OBJECT_PATH "/org/freedesktop/nm_dhcp_server" +#define NM_DHCP_HELPER_SERVER_INTERFACE_NAME "org.freedesktop.nm_dhcp_server" +#define NM_DHCP_HELPER_SERVER_METHOD_NOTIFY "Notify" + /******************************************************************************/ #endif /* __NM_DHCP_HELPER_API_H__ */ diff --git a/src/dhcp-manager/nm-dhcp-helper.c b/src/dhcp-manager/nm-dhcp-helper.c index e53fe27..9c6f69b 100644 --- a/src/dhcp-manager/nm-dhcp-helper.c +++ b/src/dhcp-manager/nm-dhcp-helper.c @@ -105,7 +105,7 @@ build_signal_parameters (void) g_free (name); } - return g_variant_new ("(a{sv})", &builder); + return g_variant_ref_sink (g_variant_new ("(a{sv})", &builder)); } static void @@ -128,7 +128,11 @@ main (int argc, char *argv[]) { gs_unref_object GDBusConnection *connection = NULL; gs_free_error GError *error = NULL; + gs_unref_variant GVariant *parameters = NULL; + gs_unref_variant GVariant *result = NULL; gboolean success = FALSE; + guint try_count = 0; + gint64 time_end; nm_g_type_init (); @@ -142,25 +146,73 @@ main (int argc, char *argv[]) goto out; } - if (!g_dbus_connection_emit_signal (connection, - NULL, - "/", - NM_DHCP_CLIENT_DBUS_IFACE, - "Event", - build_signal_parameters (), - &error)) { - g_dbus_error_strip_remote_error (error); - _LOGE ("could not send DHCP Event signal: %s", error->message); - goto out; - } + parameters = build_signal_parameters (); + + time_end = g_get_monotonic_time () + (200 * 1000L); /* retry for at most 200 milliseconds */ + +do_notify: + try_count++; + result = g_dbus_connection_call_sync (connection, + NULL, + NM_DHCP_HELPER_SERVER_OBJECT_PATH, + NM_DHCP_HELPER_SERVER_INTERFACE_NAME, + NM_DHCP_HELPER_SERVER_METHOD_NOTIFY, + parameters, + NULL, + G_DBUS_CALL_FLAGS_NONE, + 1000, + NULL, + &error); + + if (!result) { + gs_free char *s_err = NULL; + + s_err = g_dbus_error_get_remote_error (error); + if (NM_IN_STRSET (s_err, "org.freedesktop.DBus.Error.UnknownMethod")) { + gint64 remaining_time = time_end - g_get_monotonic_time (); + + /* I am not sure that a race can actually happen, as we register the object + * on the server side during GDBusServer:new-connection signal. + * + * However, there was also a race for subscribing to an event, so let's just + * do some retry. */ + if (remaining_time > 0) { + _LOGi ("failure to call notify: %s (retry %u)", error->message, try_count); + g_usleep (NM_MIN (NM_CLAMP ((gint64) (100L * (1L << try_count)), 5000, 25000), remaining_time)); + g_clear_error (&error); + goto do_notify; + } + } + _LOGW ("failure to call notify: %s (try signal via Event)", error->message); + g_clear_error (&error); + + /* for backward compatibilty, try to emit the signal. There is no stable + * API between the dhcp-helper and NetworkManager. However, while upgrading + * the NetworkManager package, a newer helper might want to notify an + * older server, which still uses the "Event". */ + if (!g_dbus_connection_emit_signal (connection, + NULL, + "/", + NM_DHCP_CLIENT_DBUS_IFACE, + "Event", + parameters, + &error)) { + g_dbus_error_strip_remote_error (error); + _LOGE ("could not send DHCP Event signal: %s", error->message); + goto out; + } + /* We were able to send the asynchronous Event. Consider that a success. */ + success = TRUE; + } else + success = TRUE; if (!g_dbus_connection_flush_sync (connection, NULL, &error)) { g_dbus_error_strip_remote_error (error); _LOGE ("could not flush D-Bus connection: %s", error->message); + success = FALSE; goto out; } - success = TRUE; out: if (!success) kill_pid (); diff --git a/src/dhcp-manager/nm-dhcp-listener.c b/src/dhcp-manager/nm-dhcp-listener.c index 7e9de49..0df4197 100644 --- a/src/dhcp-manager/nm-dhcp-listener.c +++ b/src/dhcp-manager/nm-dhcp-listener.c @@ -119,13 +119,14 @@ get_option (GVariant *options, const char *key) } static void -handle_event (GDBusConnection *connection, - const char *sender_name, - const char *object_path, - const char *interface_name, - const char *signal_name, - GVariant *parameters, - gpointer user_data) +_method_call (GDBusConnection *connection, + const char *sender, + const char *object_path, + const char *interface_name, + const char *method_name, + GVariant *parameters, + GDBusMethodInvocation *invocation, + gpointer user_data) { NMDhcpListener *self = NM_DHCP_LISTENER (user_data); char *iface = NULL; @@ -135,8 +136,12 @@ handle_event (GDBusConnection *connection, gboolean handled = FALSE; GVariant *options; + if (!nm_streq0 (interface_name, NM_DHCP_HELPER_SERVER_INTERFACE_NAME)) + g_return_if_reached (); + if (!nm_streq0 (method_name, NM_DHCP_HELPER_SERVER_METHOD_NOTIFY)) + g_return_if_reached (); if (!g_variant_is_of_type (parameters, G_VARIANT_TYPE ("(a{sv})"))) - return; + g_return_if_reached (); g_variant_get (parameters, "(@a{sv})", &options); @@ -173,6 +178,57 @@ out: g_free (pid_str); g_free (reason); g_variant_unref (options); + g_dbus_method_invocation_return_value (invocation, NULL); +} + +static guint +_dbus_connection_register_object (NMDhcpListener *self, + GDBusConnection *connection, + GError **error) +{ + static GDBusArgInfo arg_info_notify_in = { + .ref_count = -1, + .name = "data", + .signature = "a{sv}", + .annotations = NULL, + }; + static GDBusArgInfo *arg_infos_notify[] = { + &arg_info_notify_in, + NULL, + }; + static GDBusMethodInfo method_info_notify = { + .ref_count = -1, + .name = NM_DHCP_HELPER_SERVER_METHOD_NOTIFY, + .in_args = arg_infos_notify, + .out_args = NULL, + .annotations = NULL, + }; + static GDBusMethodInfo *method_infos[] = { + &method_info_notify, + NULL, + }; + static GDBusInterfaceInfo interface_info = { + .ref_count = -1, + .name = NM_DHCP_HELPER_SERVER_INTERFACE_NAME, + .methods = method_infos, + .signals = NULL, + .properties = NULL, + .annotations = NULL, + }; + + static GDBusInterfaceVTable interface_vtable = { + .method_call = _method_call, + .get_property = NULL, + .set_property = NULL, + }; + + return g_dbus_connection_register_object (connection, + NM_DHCP_HELPER_SERVER_OBJECT_PATH, + &interface_info, + &interface_vtable, + self, + NULL, + error); } static void @@ -182,17 +238,20 @@ new_connection_cb (NMBusManager *mgr, NMDhcpListener *self) { NMDhcpListenerPrivate *priv = NM_DHCP_LISTENER_GET_PRIVATE (self); - guint id; + guint registration_id; + GError *error = NULL; + + /* it is important to register the object during the new-connection signal, + * as this avoids races with the connecting object. */ + registration_id = _dbus_connection_register_object (self, connection, &error); + if (!registration_id) { + _LOGE ("failure to register %s for connection %p: %s", + NM_DHCP_HELPER_SERVER_OBJECT_PATH, connection, error->message); + g_error_free (error); + return; + } - id = g_dbus_connection_signal_subscribe (connection, - NULL, - NM_DHCP_CLIENT_DBUS_IFACE, - "Event", - NULL, - NULL, - G_DBUS_SIGNAL_FLAGS_NONE, - handle_event, self, NULL); - g_hash_table_insert (priv->connections, connection, GUINT_TO_POINTER (id)); + g_hash_table_insert (priv->connections, connection, GUINT_TO_POINTER (registration_id)); } static void @@ -205,7 +264,7 @@ dis_connection_cb (NMBusManager *mgr, id = GPOINTER_TO_UINT (g_hash_table_lookup (priv->connections, connection)); if (id) { - g_dbus_connection_signal_unsubscribe (connection, id); + g_dbus_connection_unregister_object (connection, id); g_hash_table_remove (priv->connections, connection); } } diff --git a/src/nm-bus-manager.c b/src/nm-bus-manager.c index 449de4e..270792e 100644 --- a/src/nm-bus-manager.c +++ b/src/nm-bus-manager.c @@ -221,7 +221,11 @@ private_server_new_connection (GDBusServer *server, _LOGD ("(%s) accepted connection %p on private socket", s->tag, conn); - /* Emit this for the manager */ + /* Emit this for the manager. + * + * It is essential to do this from the "new-connection" signal handler, as + * at that point no messages from the connection are yet processed + * (which avoids races with registering objects). */ g_signal_emit (s->manager, signals[PRIVATE_CONNECTION_NEW], s->detail, -- 2.7.4