device: make registration of internal device-factories more staticth/device-factory-bgo772579

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 was not 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.

4 files changed, 51 insertions, 86 deletions

diff --git a/src/Makefile.am b/src/Makefile.am
index 7d2a37f1b5..e1a0510346 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 672cf18258..f84eb7634f 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);
-
-		factory = (NMDeviceFactory *) g_object_new (ftype, NULL);
-		g_assert (factory);
-		_add_factory (factory, FALSE, "internal", callback, user_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
+
+	_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 d831af926e..2faaad9df6 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 c11e7a0b5f..35c70d2103 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
-