Re: [PATCHES] symbol visibility

[Dan Williams <dcbw redhat com>]

On Wed, 2008-08-13 at 05:55 +0200, Michael Biebl wrote:
> Hi,
> 
> while looking through the TODO list for 0.7 [1], I decided that I should 
> contribute a bit, especially as it was me, that complained about the 
> long list of exported symbols by libnm-glib and libnm-util.
> 
> So I went ahead and studied the docs a bit.
> There are basically two approaches, how to trim down the list of 
> exported symbols:
> 
> a) GCC's visibility support [2]
>   GCC extension, mostly used by C++ libraries
> 
> b) ld linker/version scripts  [3]
>   b.1) either directly via LDFLAGS -Wl,--version-script
>   b.2) or indirectly via libtools -export-symbols $FILE or 
> -export-symbols-regex, which will create a version script on the fly.
> 
> Before trying to evaluate which approach is best, I analyzed the list of 
> exported symbols (see attached *.symbols files) and checked which 
> symbols are not part of the public API (see attached *.diff files).
> 
> There are basically 5 ways
> GCC visibility
> 1.) Use -fvisibility=default (all symbols are exported by default) and 
> augment all function declarations, which shall not be exported with 
> __attribute__ ((visibility("hidden")))
> 
> Does not work, as we have autogenerated files, where we cannot add the 
> visibility attribute.
> 
> 2.) Use -fvisibility=hidden (all symbols are hidden by default) and 
> augment all function declarations, which shall be exported with 
> __attribute__ ((visibility("default"))).
> 
> Lots of code changes in the header files. To be manageable we'd have to 
> define a macro like DLLEXPORT. All installed header files would either 
> have to define such a macro or we would have to create a new header file 
> (like visibility.h) which contains this macro and all other header files 
> include that. Imho quite cumbersome
> 
> libtool
> 3.) -export-symbols-regex:
> as you can see from the attached symbols and diff files, we can't use a 
> common prefix like nm_.*. We'd have to rename private function to 
> something like _nm_* -> looks of code changes.
> 
> 4.) -export-symbols $FILE
> List all exported symbols in a separate file and pass it to libtool.
> I chatted with libtool's upstream a bit, and he recommended though, to 
> directly use a ld version script. as this provides the most flexibility,
> 
> 5.) LDFLAGS = -Wl,--version-script=$FILE
> That's the way I implemented it in the patch. I listed each exported 
> symbol explicitely (although the version script syntax would allow 
> regexes). See patch 0001 and 0002 for the details.
> Imho this is the nicest and cleanest approaches which requires the least 
> changes.
> I generated the version scripts automatically (by using objdump and 
> grepping for the symbols in the installed header files, dropping all 
> symbols which are not part of the public API), so a careful review would 
> be very appreciated.
> NOTE: I didn't enable symbol versioning as I didn't see the need for it 
> yet, I only used the version script to control the list of exported symbols
> 
> After applying 0001 and 0002, the test-crypto binary will no longer 
> build, as it uses private symbols from libnm-util. I simply removed it 
> in patch 0003. An alternative would be, to add crypto.c and 
> crypto_(gnutls|nss).c to test_crypto_SOURCES.
> If someone has a need for this binary, I could change that.
> 
> 
> Comments and feedback welcome.

I poked into visibility and talked to people who know stuff (ie Drepper)
and the version script option seems to be most highly recommended one
for a number of reasons.  So feel free to commit.

Somebody mentioned that we can also apparently prefix symbols with __ at
which point they will be exposed in static libraries (which distros
shouldn't be distributing anyway) but not in shared libraries.

I'd like to keep test_crypto around if we could, at least for a while,
so adding crypto.h and crypto.c to it's compile line would be fine with
me.

Thanks!
Dan

> 
> Michael
> P.S: As I have svn commit access now, I could commit those patches 
> myself, if you ack the changes.
> 
> 
> [1] http://live.gnome.org/NetworkManager07Release
> [2] http://gcc.gnu.org/wiki/Visibility
> [3] 
> http://www.redhat.com/docs/manuals/enterprise/RHEL-4-Manual/gnu-linker/version.html
> 
> plain text document attachment (libnm-glib0.symbols)
> dbus_glib_marshal_nm_exported_connection_BOOLEAN__POINTER_POINTER
>  dbus_glib_marshal_nm_exported_connection_VOID__BOXED_POINTER
>  dbus_glib_marshal_nm_exported_connection_VOID__STRING_BOXED_BOOLEAN_POINTER
>  dbus_glib_marshal_nm_settings_BOOLEAN__POINTER_POINTER
>  dbus_glib_nm_exported_connection_object_info
>  dbus_glib_nm_settings_object_info
>  libnm_glib_get_network_state
>  libnm_glib_init
>  libnm_glib_register_callback
>  libnm_glib_shutdown
>  libnm_glib_unregister_callback
>  nm_access_point_get_flags
>  nm_access_point_get_frequency
>  nm_access_point_get_hw_address
>  nm_access_point_get_max_bitrate
>  nm_access_point_get_mode
>  nm_access_point_get_rsn_flags
>  nm_access_point_get_ssid
>  nm_access_point_get_strength
>  nm_access_point_get_type
>  nm_access_point_get_wpa_flags
>  nm_access_point_new
>  nm_active_connection_get_connection
>  nm_active_connection_get_default
>  nm_active_connection_get_devices
>  nm_active_connection_get_scope
>  nm_active_connection_get_service_name
>  nm_active_connection_get_specific_object
>  nm_active_connection_get_state
>  nm_active_connection_get_type
>  nm_active_connection_new
>  nm_cdma_device_get_type
>  nm_cdma_device_new
>  nm_client_activate_connection
>  nm_client_deactivate_connection
>  nm_client_get_active_connections
>  nm_client_get_device_by_path
>  nm_client_get_devices
>  nm_client_get_manager_running
>  nm_client_get_state
>  nm_client_get_type
>  nm_client_new
>  nm_client_sleep
>  nm_client_wireless_get_enabled
>  nm_client_wireless_hardware_get_enabled
>  nm_client_wireless_set_enabled
>  nm_dbus_connection_get_type
>  nm_dbus_connection_new
>  nm_dbus_get_int_property
>  nm_dbus_get_object_path_property
>  nm_dbus_get_property
>  nm_dbus_get_string_property
>  nm_dbus_get_uint_property
>  nm_dbus_introspect
>  nm_dbus_set_property
>  nm_dbus_settings_get_connection_by_path
>  nm_dbus_settings_get_type
>  nm_dbus_settings_new
>  nm_dbus_settings_system_add_connection
>  nm_dbus_settings_system_get_type
>  nm_dbus_settings_system_get_unmanaged_devices
>  nm_dbus_settings_system_new
>  nm_device_ethernet_get_carrier
>  nm_device_ethernet_get_hw_address
>  nm_device_ethernet_get_speed
>  nm_device_ethernet_get_type
>  nm_device_ethernet_new
>  nm_device_get_capabilities
>  nm_device_get_dhcp4_config
>  nm_device_get_driver
>  nm_device_get_iface
>  nm_device_get_ip4_config
>  nm_device_get_managed
>  nm_device_get_product
>  nm_device_get_state
>  nm_device_get_type
>  nm_device_get_udi
>  nm_device_get_vendor
>  nm_device_new
>  nm_device_wifi_get_access_point_by_path
>  nm_device_wifi_get_access_points
>  nm_device_wifi_get_active_access_point
>  nm_device_wifi_get_bitrate
>  nm_device_wifi_get_capabilities
>  nm_device_wifi_get_hw_address
>  nm_device_wifi_get_mode
>  nm_device_wifi_get_type
>  nm_device_wifi_new
>  nm_device_wifi_set_wireless_enabled
>  nm_dhcp4_config_get_one_option
>  nm_dhcp4_config_get_options
>  nm_dhcp4_config_get_type
>  nm_dhcp4_config_new
>  nm_exported_connection_delete
>  nm_exported_connection_get_connection
>  nm_exported_connection_get_id
>  nm_exported_connection_get_type
>  nm_exported_connection_new
>  nm_exported_connection_register_object
>  nm_exported_connection_signal_removed
>  nm_exported_connection_signal_updated
>  nm_exported_connection_update
>  nm_gsm_device_get_type
>  nm_gsm_device_new
>  nm_ip4_config_get_addresses
>  nm_ip4_config_get_domains
>  nm_ip4_config_get_hostname
>  nm_ip4_config_get_nameservers
>  nm_ip4_config_get_routes
>  nm_ip4_config_get_type
>  nm_ip4_config_new
>  nm_marshal_VOID__OBJECT_OBJECT_ENUM
>  nm_marshal_VOID__OBJECT_POINTER
>  nm_marshal_VOID__OBJECT_POINTER_UINT
>  nm_marshal_VOID__OBJECT_STRING
>  nm_marshal_VOID__OBJECT_STRING_UINT
>  nm_marshal_VOID__OBJECT_UINT
>  nm_marshal_VOID__OBJECT_UINT_UINT
>  nm_marshal_VOID__POINTER_STRING
>  nm_marshal_VOID__STRING_INT
>  nm_marshal_VOID__STRING_OBJECT
>  nm_marshal_VOID__STRING_STRING
>  nm_marshal_VOID__STRING_STRING_POINTER
>  nm_marshal_VOID__STRING_STRING_STRING
>  nm_marshal_VOID__STRING_UCHAR
>  nm_marshal_VOID__STRING_UINT
>  nm_marshal_VOID__UINT_UINT
>  nm_marshal_VOID__UINT_UINT_UINT
>  nm_object_array_demarshal
>  nm_object_array_get_type
>  nm_object_cache_add
>  nm_object_cache_get
>  nm_object_cache_remove_by_object
>  nm_object_cache_remove_by_path
>  nm_object_demarshal_generic
>  nm_object_get_boolean_property
>  nm_object_get_byte_array_property
>  nm_object_get_byte_property
>  nm_object_get_connection
>  nm_object_get_double_property
>  nm_object_get_int_property
>  nm_object_get_object_path_property
>  nm_object_get_path
>  nm_object_get_property
>  nm_object_get_string_property
>  nm_object_get_type
>  nm_object_get_uint_property
>  nm_object_handle_properties_changed
>  nm_object_queue_notify
>  nm_object_set_property
>  nm_serial_device_get_bytes_received
>  nm_serial_device_get_bytes_sent
>  nm_serial_device_get_type
>  nm_settings_error_quark
>  nm_settings_get_type
>  nm_settings_list_connections
>  nm_settings_signal_new_connection
>  nm_ssid_demarshal
>  nm_ssid_get_type
>  nm_string_array_demarshal
>  nm_string_array_get_type
>  nm_uint_array_demarshal
>  nm_uint_array_get_type
>  nm_vpn_connection_get_banner
>  nm_vpn_connection_get_type
>  nm_vpn_connection_get_vpn_state
>  nm_vpn_connection_new
> plain text document attachment (libnm-glib0.symbols.diff)
> --- libnm-glib0.symbols.save	2008-08-13 04:29:17.000000000 +0200
> +++ libnm-glib0.symbols	2008-08-13 04:15:42.000000000 +0200
> @@ -1,10 +1,4 @@
> - dbus_glib_marshal_nm_exported_connection_BOOLEAN__POINTER_POINTER
> - dbus_glib_marshal_nm_exported_connection_VOID__BOXED_POINTER
> - dbus_glib_marshal_nm_exported_connection_VOID__STRING_BOXED_BOOLEAN_POINTER
> - dbus_glib_marshal_nm_settings_BOOLEAN__POINTER_POINTER
> - dbus_glib_nm_exported_connection_object_info
> - dbus_glib_nm_settings_object_info
>   libnm_glib_get_network_state
>   libnm_glib_init
>   libnm_glib_register_callback
> @@ -47,13 +41,6 @@
>   nm_client_wireless_set_enabled
>   nm_dbus_connection_get_type
>   nm_dbus_connection_new
> - nm_dbus_get_int_property
> - nm_dbus_get_object_path_property
> - nm_dbus_get_property
> - nm_dbus_get_string_property
> - nm_dbus_get_uint_property
> - nm_dbus_introspect
> - nm_dbus_set_property
>   nm_dbus_settings_get_connection_by_path
>   nm_dbus_settings_get_type
>   nm_dbus_settings_new
> @@ -87,7 +74,6 @@
>   nm_device_wifi_get_mode
>   nm_device_wifi_get_type
>   nm_device_wifi_new
> - nm_device_wifi_set_wireless_enabled
>   nm_dhcp4_config_get_one_option
>   nm_dhcp4_config_get_options
>   nm_dhcp4_config_get_type
> @@ -110,45 +96,10 @@
>   nm_ip4_config_get_routes
>   nm_ip4_config_get_type
>   nm_ip4_config_new
> - nm_marshal_VOID__OBJECT_OBJECT_ENUM
> - nm_marshal_VOID__OBJECT_POINTER
> - nm_marshal_VOID__OBJECT_POINTER_UINT
> - nm_marshal_VOID__OBJECT_STRING
> - nm_marshal_VOID__OBJECT_STRING_UINT
> - nm_marshal_VOID__OBJECT_UINT
> - nm_marshal_VOID__OBJECT_UINT_UINT
> - nm_marshal_VOID__POINTER_STRING
> - nm_marshal_VOID__STRING_INT
> - nm_marshal_VOID__STRING_OBJECT
> - nm_marshal_VOID__STRING_STRING
> - nm_marshal_VOID__STRING_STRING_POINTER
> - nm_marshal_VOID__STRING_STRING_STRING
> - nm_marshal_VOID__STRING_UCHAR
> - nm_marshal_VOID__STRING_UINT
> - nm_marshal_VOID__UINT_UINT
> - nm_marshal_VOID__UINT_UINT_UINT
> - nm_object_array_demarshal
>   nm_object_array_get_type
> - nm_object_cache_add
> - nm_object_cache_get
> - nm_object_cache_remove_by_object
> - nm_object_cache_remove_by_path
> - nm_object_demarshal_generic
> - nm_object_get_boolean_property
> - nm_object_get_byte_array_property
> - nm_object_get_byte_property
>   nm_object_get_connection
> - nm_object_get_double_property
> - nm_object_get_int_property
> - nm_object_get_object_path_property
>   nm_object_get_path
> - nm_object_get_property
> - nm_object_get_string_property
>   nm_object_get_type
> - nm_object_get_uint_property
> - nm_object_handle_properties_changed
> - nm_object_queue_notify
> - nm_object_set_property
>   nm_serial_device_get_bytes_received
>   nm_serial_device_get_bytes_sent
>   nm_serial_device_get_type
> @@ -156,11 +107,8 @@
>   nm_settings_get_type
>   nm_settings_list_connections
>   nm_settings_signal_new_connection
> - nm_ssid_demarshal
>   nm_ssid_get_type
> - nm_string_array_demarshal
>   nm_string_array_get_type
> - nm_uint_array_demarshal
>   nm_uint_array_get_type
>   nm_vpn_connection_get_banner
>   nm_vpn_connection_get_type
> plain text document attachment (libnm-glib-vpn0.symbols)
> dbus_glib_marshal_nm_vpn_plugin_BOOLEAN__BOXED_POINTER
>  dbus_glib_marshal_nm_vpn_plugin_BOOLEAN__BOXED_POINTER_POINTER
>  dbus_glib_marshal_nm_vpn_plugin_BOOLEAN__POINTER
>  dbus_glib_marshal_nm_vpn_plugin_BOOLEAN__STRING_POINTER
>  dbus_glib_nm_vpn_plugin_object_info
>  nm_vpn_plugin_disconnect
>  nm_vpn_plugin_error_get_type
>  nm_vpn_plugin_error_quark
>  nm_vpn_plugin_failure
>  nm_vpn_plugin_get_connection
>  nm_vpn_plugin_get_state
>  nm_vpn_plugin_get_type
>  nm_vpn_plugin_set_ip4_config
>  nm_vpn_plugin_set_login_banner
>  nm_vpn_plugin_set_state
>  nm_vpn_plugin_ui_interface_export
>  nm_vpn_plugin_ui_interface_get_capabilities
>  nm_vpn_plugin_ui_interface_get_suggested_name
>  nm_vpn_plugin_ui_interface_get_type
>  nm_vpn_plugin_ui_interface_import
>  nm_vpn_plugin_ui_interface_ui_factory
>  nm_vpn_plugin_ui_widget_interface_get_type
>  nm_vpn_plugin_ui_widget_interface_get_widget
>  nm_vpn_plugin_ui_widget_interface_update_connection
> plain text document attachment (libnm-glib-vpn0.symbols.diff)
> --- libnm-glib-vpn0.symbols.save	2008-08-13 04:29:17.000000000 +0200
> +++ libnm-glib-vpn0.symbols	2008-08-13 04:17:49.000000000 +0200
> @@ -1,9 +1,4 @@
> - dbus_glib_marshal_nm_vpn_plugin_BOOLEAN__BOXED_POINTER
> - dbus_glib_marshal_nm_vpn_plugin_BOOLEAN__BOXED_POINTER_POINTER
> - dbus_glib_marshal_nm_vpn_plugin_BOOLEAN__POINTER
> - dbus_glib_marshal_nm_vpn_plugin_BOOLEAN__STRING_POINTER
> - dbus_glib_nm_vpn_plugin_object_info
>   nm_vpn_plugin_disconnect
>   nm_vpn_plugin_error_get_type
>   nm_vpn_plugin_error_quark
> plain text document attachment (libnm-util0.symbols)
> crypto_decrypt
>  crypto_deinit
>  crypto_get_private_key
>  crypto_init
>  crypto_load_and_verify_certificate
>  crypto_md5_hash
>  crypto_verify_cert
>  nm_connection_add_setting
>  nm_connection_clear_secrets
>  nm_connection_compare
>  nm_connection_create_setting
>  nm_connection_dump
>  nm_connection_duplicate
>  nm_connection_error_get_type
>  nm_connection_error_quark
>  nm_connection_for_each_setting_value
>  nm_connection_get_path
>  nm_connection_get_scope
>  nm_connection_get_setting
>  nm_connection_get_setting_by_name
>  nm_connection_get_type
>  nm_connection_lookup_setting_type
>  nm_connection_lookup_setting_type_by_quark
>  nm_connection_need_secrets
>  nm_connection_new
>  nm_connection_new_from_hash
>  nm_connection_remove_setting
>  nm_connection_replace_settings
>  nm_connection_set_path
>  nm_connection_set_scope
>  nm_connection_to_hash
>  nm_connection_update_secrets
>  nm_connection_verify
>  nm_crypto_error_quark
>  nm_param_spec_specialized
>  nm_param_spec_specialized_get_type
>  nm_setting_802_1x_error_get_type
>  nm_setting_802_1x_error_quark
>  nm_setting_802_1x_get_type
>  nm_setting_802_1x_new
>  nm_setting_802_1x_set_ca_cert
>  nm_setting_802_1x_set_client_cert
>  nm_setting_802_1x_set_phase2_ca_cert
>  nm_setting_802_1x_set_phase2_client_cert
>  nm_setting_802_1x_set_phase2_private_key
>  nm_setting_802_1x_set_private_key
>  nm_setting_cdma_error_get_type
>  nm_setting_cdma_error_quark
>  nm_setting_cdma_get_type
>  nm_setting_cdma_new
>  nm_setting_clear_secrets
>  nm_setting_compare
>  nm_setting_connection_error_get_type
>  nm_setting_connection_error_quark
>  nm_setting_connection_get_type
>  nm_setting_connection_new
>  nm_setting_duplicate
>  nm_setting_enumerate_values
>  nm_setting_from_hash
>  nm_setting_get_name
>  nm_setting_get_type
>  nm_setting_gsm_error_get_type
>  nm_setting_gsm_error_quark
>  nm_setting_gsm_get_type
>  nm_setting_gsm_new
>  nm_setting_ip4_config_error_get_type
>  nm_setting_ip4_config_error_quark
>  nm_setting_ip4_config_get_type
>  nm_setting_ip4_config_new
>  nm_setting_ip6_config_error_get_type
>  nm_setting_ip6_config_error_quark
>  nm_setting_ip6_config_get_type
>  nm_setting_ip6_config_new
>  nm_setting_need_secrets
>  nm_setting_ppp_error_get_type
>  nm_setting_ppp_error_quark
>  nm_setting_ppp_get_type
>  nm_setting_ppp_new
>  nm_setting_pppoe_error_get_type
>  nm_setting_pppoe_error_quark
>  nm_setting_pppoe_get_type
>  nm_setting_pppoe_new
>  nm_setting_register
>  nm_setting_serial_error_get_type
>  nm_setting_serial_error_quark
>  nm_setting_serial_get_type
>  nm_setting_serial_new
>  nm_setting_to_hash
>  nm_setting_to_string
>  nm_setting_unregister
>  nm_setting_update_secrets
>  nm_setting_verify
>  nm_setting_vpn_error_get_type
>  nm_setting_vpn_error_quark
>  nm_setting_vpn_get_type
>  nm_setting_vpn_new
>  nm_setting_wired_error_get_type
>  nm_setting_wired_error_quark
>  nm_setting_wired_get_type
>  nm_setting_wired_new
>  nm_setting_wireless_ap_security_compatible
>  nm_setting_wireless_error_get_type
>  nm_setting_wireless_error_quark
>  nm_setting_wireless_get_type
>  nm_setting_wireless_new
>  nm_setting_wireless_security_error_get_type
>  nm_setting_wireless_security_error_quark
>  nm_setting_wireless_security_get_type
>  nm_setting_wireless_security_new
>  nm_utils_escape_ssid
>  nm_utils_garray_to_string
>  nm_utils_gvalue_hash_dup
>  nm_utils_ip4_addresses_from_gvalue
>  nm_utils_ip4_addresses_to_gvalue
>  nm_utils_ip4_netmask_to_prefix
>  nm_utils_ip4_prefix_to_netmask
>  nm_utils_ip4_routes_from_gvalue
>  nm_utils_ip4_routes_to_gvalue
>  nm_utils_ip6_addresses_from_gvalue
>  nm_utils_ip6_addresses_to_gvalue
>  nm_utils_ip6_dns_from_gvalue
>  nm_utils_ip6_dns_to_gvalue
>  nm_utils_is_empty_ssid
>  nm_utils_register_value_transformations
>  nm_utils_same_ssid
>  nm_utils_security_valid
>  nm_utils_slist_free
>  nm_utils_ssid_to_utf8
>  nm_utils_string_in_list
>  nm_utils_string_list_contains
>  nm_utils_string_slist_validate
> plain text document attachment (libnm-util0.symbols.diff)
> --- libnm-util0.symbols.save	2008-08-13 04:29:17.000000000 +0200
> +++ libnm-util0.symbols	2008-08-13 04:26:29.000000000 +0200
> @@ -1,11 +1,4 @@
> - crypto_decrypt
> - crypto_deinit
> - crypto_get_private_key
> - crypto_init
> - crypto_load_and_verify_certificate
> - crypto_md5_hash
> - crypto_verify_cert
>   nm_connection_add_setting
>   nm_connection_clear_secrets
>   nm_connection_compare
> @@ -32,9 +25,6 @@
>   nm_connection_to_hash
>   nm_connection_update_secrets
>   nm_connection_verify
> - nm_crypto_error_quark
> - nm_param_spec_specialized
> - nm_param_spec_specialized_get_type
>   nm_setting_802_1x_error_get_type
>   nm_setting_802_1x_error_quark
>   nm_setting_802_1x_get_type
> _______________________________________________
> NetworkManager-list mailing list
> NetworkManager-list gnome org
> http://mail.gnome.org/mailman/listinfo/networkmanager-list