From 2c26e3e7f9c2bb9afbf5d59a1ccc5622034e5cce Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 7 Oct 2016 16:48:41 +0200 Subject: [PATCH] device: make registration of internal device-factories more explicit Internal device types are a static thing. Let's not do a constructor function to register the device factory. This gets rid of all attribute((constructor)) functions inside NetworkManager core. That is desired, because we don't want to run code before main(). For example, at that point logging is not yet initialized, but with code that runs before main() it is hard to ensure that we don't log anything yet. --- src/Makefile.am | 57 ++++++++++++--------------------- src/devices/nm-device-factory.c | 44 +++++++++++++++---------- src/devices/nm-device-factory.h | 18 ++--------- src/tests/Makefile.am | 16 --------- 4 files changed, 50 insertions(+), 85 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 7d2a37f1b..e1a051034 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -259,38 +259,6 @@ libNetworkManagerBase_la_LIBADD = \ ############################################################################### -# These source files have a attribute((constructor)) to register their factories. -# This gets stripped out from the resulting binary if we add them to libNetworkManager.la. -# Instead, add them to the binary. An alternative would be to link with --as-needed. - -nm_device_sources = \ - devices/nm-device-bond.c \ - devices/nm-device-bridge.c \ - devices/nm-device-ethernet.c \ - devices/nm-device-infiniband.c \ - devices/nm-device-ip-tunnel.c \ - devices/nm-device-macvlan.c \ - devices/nm-device-tun.c \ - devices/nm-device-veth.c \ - devices/nm-device-vlan.c \ - devices/nm-device-vxlan.c \ - $(NULL) - -nm_device_headers = \ - devices/nm-device-bond.h \ - devices/nm-device-bridge.h \ - devices/nm-device-ethernet.h \ - devices/nm-device-infiniband.h \ - devices/nm-device-ip-tunnel.h \ - devices/nm-device-macvlan.h \ - devices/nm-device-tun.h \ - devices/nm-device-veth.h \ - devices/nm-device-vlan.h \ - devices/nm-device-vxlan.h \ - $(NULL) - -############################################################################### - libNetworkManager_la_SOURCES = \ \ nm-checkpoint-manager.c \ @@ -313,6 +281,27 @@ libNetworkManager_la_SOURCES = \ devices/nm-device-logging.h \ devices/nm-device-private.h \ \ + devices/nm-device-bond.c \ + devices/nm-device-bond.h \ + devices/nm-device-bridge.c \ + devices/nm-device-bridge.h \ + devices/nm-device-ethernet.c \ + devices/nm-device-ethernet.h \ + devices/nm-device-infiniband.c \ + devices/nm-device-infiniband.h \ + devices/nm-device-ip-tunnel.c \ + devices/nm-device-ip-tunnel.h \ + devices/nm-device-macvlan.c \ + devices/nm-device-macvlan.h \ + devices/nm-device-tun.c \ + devices/nm-device-tun.h \ + devices/nm-device-veth.c \ + devices/nm-device-veth.h \ + devices/nm-device-vlan.c \ + devices/nm-device-vlan.h \ + devices/nm-device-vxlan.c \ + devices/nm-device-vxlan.h \ + \ dhcp-manager/nm-dhcp-client.c \ dhcp-manager/nm-dhcp-client.h \ dhcp-manager/nm-dhcp-client-logging.h \ @@ -513,10 +502,6 @@ libNetworkManagerTest_la_LIBADD = \ ############################################################################### NetworkManager_SOURCES = \ - \ - $(nm_device_sources) \ - $(nm_device_headers) \ - \ main-utils.c \ main-utils.h \ main.c diff --git a/src/devices/nm-device-factory.c b/src/devices/nm-device-factory.c index bdc893c49..1f6530086 100644 --- a/src/devices/nm-device-factory.c +++ b/src/devices/nm-device-factory.c @@ -254,21 +254,12 @@ nm_device_factory_class_init (NMDeviceFactoryClass *klass) /*****************************************************************************/ -static GSList *internal_types = NULL; static GHashTable *factories_by_link = NULL; static GHashTable *factories_by_setting = NULL; -void -_nm_device_factory_internal_register_type (GType factory_type) -{ - g_return_if_fail (g_slist_find (internal_types, GUINT_TO_POINTER (factory_type)) == NULL); - internal_types = g_slist_prepend (internal_types, GUINT_TO_POINTER (factory_type)); -} - static void __attribute__((destructor)) _cleanup (void) { - g_clear_pointer (&internal_types, g_slist_free); g_clear_pointer (&factories_by_link, g_hash_table_unref); g_clear_pointer (&factories_by_setting, g_hash_table_unref); } @@ -481,12 +472,22 @@ _add_factory (NMDeviceFactory *factory, return TRUE; } +static void +_load_internal_factory (GType factory_gtype, + NMDeviceFactoryManagerFactoryFunc callback, + gpointer user_data) +{ + NMDeviceFactory *factory; + + factory = (NMDeviceFactory *) g_object_new (factory_gtype, NULL); + _add_factory (factory, FALSE, "internal", callback, user_data); +} + void nm_device_factory_manager_load_factories (NMDeviceFactoryManagerFactoryFunc callback, gpointer user_data) { NMDeviceFactory *factory; - const GSList *iter; GError *error = NULL; char **path, **paths; @@ -496,14 +497,23 @@ nm_device_factory_manager_load_factories (NMDeviceFactoryManagerFactoryFunc call factories_by_link = g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL, g_object_unref); factories_by_setting = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, g_object_unref); - /* Register internal factories first */ - for (iter = internal_types; iter; iter = iter->next) { - GType ftype = (GType) GPOINTER_TO_SIZE (iter->data); +#define _ADD_INTERNAL(get_type_fcn) \ + G_STMT_START { \ + GType get_type_fcn (void); \ + _load_internal_factory (get_type_fcn (), \ + callback, user_data); \ + } G_STMT_END - factory = (NMDeviceFactory *) g_object_new (ftype, NULL); - g_assert (factory); - _add_factory (factory, FALSE, "internal", callback, user_data); - } + _ADD_INTERNAL (nm_bond_device_factory_get_type); + _ADD_INTERNAL (nm_bridge_device_factory_get_type); + _ADD_INTERNAL (nm_ethernet_device_factory_get_type); + _ADD_INTERNAL (nm_infiniband_device_factory_get_type); + _ADD_INTERNAL (nm_ip_tunnel_device_factory_get_type); + _ADD_INTERNAL (nm_macvlan_device_factory_get_type); + _ADD_INTERNAL (nm_tun_device_factory_get_type); + _ADD_INTERNAL (nm_veth_device_factory_get_type); + _ADD_INTERNAL (nm_vlan_device_factory_get_type); + _ADD_INTERNAL (nm_vxlan_device_factory_get_type); paths = read_device_factory_paths (); if (!paths) diff --git a/src/devices/nm-device-factory.h b/src/devices/nm-device-factory.h index d831af926..2faaad9df 100644 --- a/src/devices/nm-device-factory.h +++ b/src/devices/nm-device-factory.h @@ -233,21 +233,9 @@ extern const char *_nm_device_factory_no_default_settings[]; NMDeviceFactoryClass parent; \ } NM##mixed##DeviceFactoryClass; \ \ - static GType nm_##lower##_device_factory_get_type (void); \ + GType nm_##lower##_device_factory_get_type (void); \ \ - G_DEFINE_TYPE_WITH_CODE (NM##mixed##DeviceFactory, nm_##lower##_device_factory, NM_TYPE_DEVICE_FACTORY, \ - _nm_device_factory_internal_register_type (g_define_type_id);) \ - \ - /* Use a module constructor to register the factory's GType at load \ - * time, which then calls _nm_device_factory_internal_register_type() \ - * to register the factory's GType with the Manager. \ - */ \ - static void __attribute__((constructor)) \ - register_device_factory_internal_##lower (void) \ - { \ - nm_g_type_init (); \ - g_type_ensure (NM_TYPE_##upper##_DEVICE_FACTORY); \ - } \ + G_DEFINE_TYPE (NM##mixed##DeviceFactory, nm_##lower##_device_factory, NM_TYPE_DEVICE_FACTORY) \ \ NM_DEVICE_FACTORY_DECLARE_TYPES(st_code) \ \ @@ -265,8 +253,6 @@ extern const char *_nm_device_factory_no_default_settings[]; dfi_code \ } -void _nm_device_factory_internal_register_type (GType factory_type); - /************************************************************************** * PRIVATE FACTORY FUNCTIONS - for factory consumers (eg, NMManager). **************************************************************************/ diff --git a/src/tests/Makefile.am b/src/tests/Makefile.am index c11e7a0b5..35c70d210 100644 --- a/src/tests/Makefile.am +++ b/src/tests/Makefile.am @@ -168,19 +168,3 @@ TESTS = \ test-wired-defname \ test-utils - -if ENABLE_TESTS - -check-local: - @for t in bond bridge ethernet infiniband ip_tunnel macvlan tun veth vlan vxlan; do \ - # Ensure the device subclass factory registration constructors exist \ - # which could inadvertently break if src/Makefile.am gets changed \ - if ! LC_ALL=C nm $(top_builddir)/src/NetworkManager | LC_ALL=C grep -q "register_device_factory_internal_$$t" ; then \ - echo "Testing device factory symbols... FAILED" ; \ - exit 1 ; \ - fi \ - done - @echo "Testing device factory symbols... PASSED" - -endif -