GNOME Bugzilla – Bug 313596
GConf exports private symbols
Last modified: 2018-08-17 13:59:32 UTC
A large percentage of gconf functions are not well documented. Many do not include docs for function arguments or return codes. Bug 147758, bug 159167, and bug 159411 highlight certain sections but there are these types of problems on pretty much every page of the gtk-docs. The first bug has a patch and the other two bugs could probably be closed as duplicates of this bug. It's probably better to address all these problems in one go. The gconf_handle_oaf_exception method is in the docs, but does not seem to be an exported symbol of the library. The following interfaces have been added to GConf since GNOME 2.0 but do not seem to have "Since:" tags in the gtk-docs: ConfigServer2_get_database_for_addresses POA_ConfigServer2__fini POA_ConfigServer2__init _ORBIT_skel_small_ConfigServer2_get_database_for_addresses gconf_address_list_free gconf_address_list_get_persistent_name gconf_change_set_get_type gconf_client_notify gconf_client_recursive_unset gconf_engine_get_for_addresses gconf_engine_get_local_for_addresses gconf_persistent_name_get_address_list gconf_sources_add_listener gconf_sources_is_affected gconf_sources_remove_listener gconf_sources_set_notify_func The GConf library exports the following interfaces that do not seem to have documentation. Some of these probably need to be removed from the GConf symbol list (such as _ORBIT* functions), while others need docs: ConfigDatabase2_all_entries_with_schema_name ConfigDatabase2_lookup_with_schema_name ConfigDatabase3_add_listener_with_properties ConfigDatabase3_recursive_unset ConfigDatabase_add_listener ConfigDatabase_all_dirs ConfigDatabase_all_entries ConfigDatabase_batch_change ConfigDatabase_batch_lookup ConfigDatabase_clear_cache ConfigDatabase_dir_exists ConfigDatabase_lookup ConfigDatabase_lookup_default_value ConfigDatabase_lookup_with_locale ConfigDatabase_remove_dir ConfigDatabase_remove_listener ConfigDatabase_set ConfigDatabase_set_schema ConfigDatabase_sync ConfigDatabase_synchronous_sync ConfigDatabase_unset ConfigDatabase_unset_with_locale ConfigListener_drop_all_caches ConfigListener_invalidate_cached_values ConfigListener_notify ConfigListener_ping ConfigListener_update_listener ConfigServer2_get_database_for_addresses ConfigServer_add_client ConfigServer_get_database ConfigServer_get_default_database ConfigServer_ping ConfigServer_remove_client ConfigServer_shutdown POA_ConfigDatabase2__fini POA_ConfigDatabase2__init POA_ConfigDatabase3__fini POA_ConfigDatabase3__init POA_ConfigDatabase__fini POA_ConfigDatabase__init POA_ConfigListener__fini POA_ConfigListener__init POA_ConfigServer2__fini POA_ConfigServer2__init POA_ConfigServer__fini POA_ConfigServer__init _ORBIT_skel_small_ConfigDatabase2_all_entries_with_schema_name _ORBIT_skel_small_ConfigDatabase2_lookup_with_schema_name _ORBIT_skel_small_ConfigDatabase3_add_listener_with_properties _ORBIT_skel_small_ConfigDatabase3_recursive_unset _ORBIT_skel_small_ConfigDatabase_add_listener _ORBIT_skel_small_ConfigDatabase_all_dirs _ORBIT_skel_small_ConfigDatabase_all_entries _ORBIT_skel_small_ConfigDatabase_batch_change _ORBIT_skel_small_ConfigDatabase_batch_lookup _ORBIT_skel_small_ConfigDatabase_clear_cache _ORBIT_skel_small_ConfigDatabase_dir_exists _ORBIT_skel_small_ConfigDatabase_lookup _ORBIT_skel_small_ConfigDatabase_lookup_default_value _ORBIT_skel_small_ConfigDatabase_lookup_with_locale _ORBIT_skel_small_ConfigDatabase_remove_dir _ORBIT_skel_small_ConfigDatabase_remove_listener _ORBIT_skel_small_ConfigDatabase_set _ORBIT_skel_small_ConfigDatabase_set_schema _ORBIT_skel_small_ConfigDatabase_sync _ORBIT_skel_small_ConfigDatabase_synchronous_sync _ORBIT_skel_small_ConfigDatabase_unset _ORBIT_skel_small_ConfigDatabase_unset_with_locale _ORBIT_skel_small_ConfigListener_drop_all_caches _ORBIT_skel_small_ConfigListener_invalidate_cached_values _ORBIT_skel_small_ConfigListener_notify _ORBIT_skel_small_ConfigListener_ping _ORBIT_skel_small_ConfigListener_update_listener _ORBIT_skel_small_ConfigServer2_get_database_for_addresses _ORBIT_skel_small_ConfigServer_add_client _ORBIT_skel_small_ConfigServer_get_database _ORBIT_skel_small_ConfigServer_get_default_database _ORBIT_skel_small_ConfigServer_ping _ORBIT_skel_small_ConfigServer_remove_client _ORBIT_skel_small_ConfigServer_shutdown _gconf_init_i18n gconf_CORBA_Object_equal gconf_CORBA_Object_hash gconf_activate_server gconf_address_flags gconf_address_list_free gconf_address_list_get_persistent_name gconf_blow_away_locks (poorly documented) gconf_change_set_get_type gconf_change_set_get_user_data gconf_change_set_set_user_data gconf_clear_cache (poorly documented) gconf_client_error_handling_mode_get_type gconf_client_get_type gconf_client_key_is_writable gconf_client_notify gconf_client_preload_type_get_type gconf_client_recursive_unset gconf_corba_schema_from_gconf_schema gconf_corba_value_from_gconf_value gconf_daemon_blow_away_locks gconf_debug_shutdown gconf_double_to_string gconf_engine_get_for_addresses gconf_engine_get_fuller gconf_engine_get_local_for_addresses gconf_engine_get_user_data gconf_engine_key_is_writable gconf_engine_pop_owner_usage gconf_engine_push_owner_usage gconf_engine_recursive_unset gconf_engine_remove_dir gconf_engine_set_owner gconf_engine_set_user_data gconf_entry_copy gconf_entry_equal gconf_entry_get_is_writable gconf_entry_new gconf_entry_ref gconf_entry_set_is_writable gconf_entry_set_value gconf_entry_unref gconf_error_get_type gconf_error_quark gconf_escape_key gconf_fill_corba_schema_from_gconf_schema gconf_fill_corba_value_from_gconf_value gconf_get_current_lock_holder gconf_get_daemon_dir gconf_get_daemon_ior gconf_get_lock gconf_get_lock_dir gconf_get_lock_or_current_holder gconf_in_daemon_mode gconf_invalid_corba_value gconf_listeners_foreach gconf_listeners_get_data gconf_listeners_remove_if gconf_marshal_VOID__STRING_POINTER gconf_object_to_string gconf_orb_get gconf_orb_release gconf_persistent_name_get_address_list gconf_release_lock gconf_schema_steal_default_value gconf_schema_validate gconf_set_daemon_ior gconf_sources_add_listener gconf_sources_clear_cache gconf_sources_is_affected gconf_sources_recursive_unset gconf_sources_remove_listener gconf_sources_set_notify_func gconf_string_to_double gconf_synchronous_sync gconf_unescape_key gconf_unique_key gconf_use_local_locks gconf_value_compare gconf_value_set_string_nocopy gconf_value_steal_list gconf_value_steal_schema gconf_value_steal_string gconf_value_type_get_type gconf_value_validate
For reference, the new functions were added in these releases. The gconf_change_set_get_type function probably doesn't need docs since "get_type" functions are internal and not documented. 2.7.1 gconf_address_list_free 2.7.1 gconf_address_list_get_persistent_name 2.4.0 gconf_client_notify 2.3.3 gconf_client_recursive_unset 2.3.3 gconf_engine_get_for_addresses 2.7.1 gconf_engine_get_local_for_addresses 2.7.1 gconf_persistent_name_get_address_list 2.7.1 gconf_sources_add_listener 2.7.1 gconf_sources_is_affected 2.7.1 gconf_sources_remove_listener 2.7.1 gconf_sources_set_notify_func
Note that is normal to list the first stable release an interface was added so add "Since: 2.8" for an interface added in 2.7.1 for example.
I'm adding a sample patch which provides documentation support for two of the functions (gconf_client_preload_type_get_type() and gconf_client_key_is_writable() ). Just wanted to ensure that i'm proceeding in the right direction. Kindly comment on the below patch so that i can proceed with the work. Also, please comment on whether it is better to provide the documentation in the '.c and .h' files or the '.sgml' file.
Created attachment 52312 [details] [review] A sample patch which provides doc for two functions
The patch looks good to me except for one thing. I notice that _get_type interfaces in GNOME in general are undocumented. I believe this is because these functions are typically not called directory and are instead called by the TYPE macros that call the function (note the definition of the TYPE macros in the header files). It's probably a good idea to make sure that the TYPE macros are in the gtk-docs, but not the *_get_type functions themselves.
*** Bug 159411 has been marked as a duplicate of this bug. ***
*** Bug 159167 has been marked as a duplicate of this bug. ***
I looked into the /usr/include directory to sort out the list of functions that are present in the gconf header files that may be used by other applications. Out of the 162 functions present in the above list, only 28 are actually exported this way. I'm attaching a patch which provides documentation for the 28 functions. Kindly give your comments.
Created attachment 52553 [details] [review] Patch to provide documentation
I'm adding a complete patch which provides documentation for all the functions exported by gconf, including those which are exported and not present in the above mentioned list. This when combined with patch for Bug 147758 would ensure that all APIs exported by gconf are documented.
Created attachment 52976 [details] [review] Patch to provide complete documentation.
Am I missing something here? I expected the docs to be written in the actual .c files. These .sgml files are generated from the inline docs in the source code, no?
Documentation on functions can be placed inside the source files or it can be added to the template sgml files in the tmpl directory. Kindly refer the following link. http://mail.gnome.org/archives/gtk-doc-list/2005-February/msg00020.html I followed the second approach since most of the existing documentation for gconf had been written that way.
Right, I was confused then :-)
Can this patch be applied then?
Dinoop: sorry about the delay, please go ahead and commit. Thanks Just a couple of comments from a quick glance over the patch: +<!-- ##### FUNCTION gconf_client_notify ##### --> +<para> +Obtains the #GConfEntry associated with the #GConfClient using gconf_client_get_entry(). +If the entry is not <symbol>NULL </symbol> it emits the #value_changed signal by calling +gconf_client_value_changed() and notifies the listeners associated with the client,if any. +</para> I wouldn't explain it this way - the fact that the code use get_entry() and value_changed() isn't really of importance. Something like "Emits the "value-changed" signal and notifies listeners as if @key had been changed" <!-- ##### FUNCTION gconf_client_set_global_default_error_handler ##### --> <para> - +Set @func as the default error handler for the #GConfClient. But under what conditions does @func get called? Why would one set it?
Mark: Thanks for reviewing the patch. I'll commit the patch after making the changes you suggested.
Committed the patch to provide documentation. Leaving the bug open since there are some symbols exported by gconf library which needs to be made private.
Created attachment 64844 [details] [review] Patch to make gconf not to export private symbols Attaching a patch which makes gconf not to export all the private symbols. The patch uses the '-export-symbols-regex' option so that all symbols which start with '_' are not exported and remain private to the library. Have also prefixed all the private symbols in gconf library with '_' so that they are not exported. But even after doing the above, the private symbols show up as exported ones in the 'nm' output in my system. Brian confirmed that it works fine in his system. Request some one to test the attached patch.
Created attachment 64845 [details] List of functions which the patch makes private by prefixing '_' .
*** Bug 62242 has been marked as a duplicate of this bug. ***
Changing components since this now affects code and not docs.
So there are a few problems with attachment 64845 [details]. The first problem (and the reason why it doesn't work as mentioned in comment 19) is that the gmodule-2.0.pc file has -export-dynamic in it. GConf should be changed to check for gmodule-no-export-2.0 instead. The second problem is that the various gconf binaries (gconftool-2, gconfd-2, etc) link against libgconf and use the internal symbols (which are no longer exported). The internal symbols should probably be moved to a private static convenience library, so that those programs can still use them.
the extra memory usage etc. caused by not sharing the code with gconfd (which is a persistent process) may well outweigh any efficiency improvement from making the symbols private, no? In any case this doesn't seem worth spending time on, vs. getting rid of gconf entirely. gconf can't be cleaned up "in place" to any significant degree and this is the least of the problems...
I don't think the change was motivated by a desire to make things more efficient, for what it's worth, but I could be wrong. On the other hand, you're right that there could be a pretty big jump in memory usage on large, multiuser systems if we go the static lib route. A couple of ways to address that would be to continue to export the functions that gconfd uses or to move those functions to a libgconf-private library. Whether the change is worth spending time on is a different question entirely, of course.
GConf has been deprecated since 2011. GConf is not under active development anymore. Its codebase has been archived: https://gitlab.gnome.org/Archive/gconf/commits/master dconf and gsettings are its successors. See https://developer.gnome.org/gio/stable/ch34.html and https://developer.gnome.org/GSettings/ for porting info. Closing this report as WONTFIX as part of Bugzilla Housekeeping to reflect reality. Feel free to open a task in GNOME Gitlab if the issue described in this task still applies to a recent + supported version of dconf/gsettings. Thanks!