After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 307356 - Visibility of public symbols
Visibility of public symbols
Status: RESOLVED FIXED
Product: libgoffice
Classification: Other
Component: General
unspecified
Other All
: Normal normal
: ---
Assigned To: Jody Goldberg
Jody Goldberg
Depends on:
Blocks:
 
 
Reported: 2005-06-12 09:05 UTC by Ivan Wong
Modified: 2006-02-21 19:42 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed patch (16.41 KB, patch)
2005-07-10 19:51 UTC, Ivan Wong
none Details | Review
Revised (15.78 KB, patch)
2005-07-12 09:42 UTC, Ivan Wong
none Details | Review
Just testing. (2.92 KB, patch)
2005-12-20 17:24 UTC, Olav Vitters
none Details | Review
efew (2.92 KB, patch)
2005-12-20 18:11 UTC, Olav Vitters
none Details | Review
revised (22.95 KB, patch)
2005-12-20 18:12 UTC, Olav Vitters
none Details | Review
revised (22.00 KB, patch)
2006-01-23 18:20 UTC, Ivan Wong
none Details | Review

Description Ivan Wong 2005-06-12 09:05:12 UTC
Currently we use --export-all-symbols when linking the library for Win32. It
doesn't only export symbols which are not supposed to be public, but it also
prohibits us to link to many static libraries compiled by Microsoft's compiler,
as they often contain unexportable symbols.

We can either borrow the gtk+'s approach or use __declspec(dllimport). For the
former one, it also utilizes symbol visibility of gcc so that unixish builds are
also benifited. However, it requires us to maintain an extra symbol list file
(may be goffice.symbols). For the later one, it only works for Win32 but we just
have to define a macro like this:

#ifndef GO_API
#  ifdef WIN32
#    ifdef GOFFICE_STATIC_COMPILATION
#      define GO_API extern
#    else /* !GOFFICE_STATIC_COMPILATION */
#      ifdef GOFFICE_COMPILATION
#        ifdef DLL_EXPORT
#          define GO_API __declspec(dllexport)
#        else /* !DLL_EXPORT */
#          define GO_API extern
#        endif /* !DLL_EXPORT */
#      else /* !GOFFICE_COMPILATION */
#        define GO_API extern __declspec(dllimport)
#      endif /* !GOFFICE_COMPILATION */
#    endif /* !GOFFICE_STATIC_COMPILATION */
#  else /* !WIN32 */
#    ifndef GOFFICE_COMPILATION
#      define GO_API extern
#    else
#      define GO_API
#    endif /* !GOFFICE_COMPILATION */
#  endif /* !WIN32 */
#endif /* GO_API */

and prepend GO_API to every public function or symbol:

GO_API void libgoffice_init     (void);
GO_API long double go_ninfl;

The biggest advantage of this approach is that we don't need to update
goffice.symbols when we change the API.
Comment 1 Morten Welinder 2005-06-13 15:50:26 UTC
Oh, bother.  I would hate to have to do this, but I guess we have no real
choice.

1. Does that macro above really work for goffice plugins?  I would assume that
   they would need to import stuff from libgoffice.so but export a select few
   of their own symbols.

2. GO_API could be simplified by simply having

   #ifndef GO_API
   #define GO_API extern
   #endif

   (which should work for all non-win32 cases) as a catch all following the
   main definition.
Comment 2 Ivan Wong 2005-06-13 16:33:14 UTC
1. For plugins I assume they use G_MODULE_EXPORT to export their own symbols.
2. The whole macro is a copy of GDK_PIXBUF_VAR from gdk-pixbuf-features.h, I am 
not sure why they made an extra ifdef for non-win32 cases.
Comment 3 Jody Goldberg 2005-06-18 18:52:26 UTC
re: plugins.
Yes, I'd like to see them use G_MODULE_EXPORT only.  They really should not be
exposing much api.

GO_API seems ugly.  Having that infront of every every api entry that is to be
exposed to plugins is not reasonable.  As problematic as .defs files are, they
still seem like a more comfortable solution.
Comment 4 Ivan Wong 2005-07-10 19:51:13 UTC
Created attachment 48920 [details] [review]
proposed patch

Based on the idea from Jody, here's a simpler solution:

1. For every source directory of a library, pass all the public headers plus
goffice-config.h (with all #include removed) to the C preprocessor.
2. Pass the result from 1. to a perl script which extracts all the public
functions and variables.
3. Concat all from step 2. to produce a single .def file at the top directory
of the library.

I have tried my best to ensure that there's no false positive and true
negative, with the help of the .def file generated by --export-all-symbols.
Right now we have 971 public symbols.

What we gain with this change:

1. The possibility to link to a static library that has unexportable symbols
and is built by MSVC, e.g. htmlhelp, urlmon, etc.
2. We need not to keep a .def file manually. MSVC has no option similar to
--export-all-symbols.
Comment 5 Ivan Wong 2005-07-12 09:42:15 UTC
Created attachment 49006 [details] [review]
Revised

As discussed with Jody on IRC, here are some changes:

1. remove all `#if 0`ed fucntion declarations
2. use a static name (local.def) for all .def files.
3. minimize per Makefile changes to +1 line only

However, it sounds those .def make rules still have to be stored in a separated
file (goffice-win32.mk), since Makefile.am of cut-n-paste libs don't include
goffice.mk (and should include goffice-win32.mk explicitly).
Comment 6 Jean Bréfort 2005-08-07 16:44:21 UTC
I have started some work on components based on plugins. I wrote one of them in
C++. G_MODULE_EXPORT does NOT work in that case apparently.
Comment 7 Ivan Wong 2005-08-07 17:43:00 UTC
Jean: have you put G_BEGIN_DECLS and G_END_DECLS around the declarations of 
your functions?
Comment 8 Jean Bréfort 2005-08-08 10:19:34 UTC
Yes, as far as I remember. I'll make more tests
Comment 9 Jean Bréfort 2005-08-09 10:00:17 UTC
In the end, what does not work is adding
GOFFICE_PLUGIN_MODULE_HEADER;
in a c++ file, even with G_BEGIN_DECS..G_END_DECLS.

I had to add the extern keyword to have the symbols exported (I do not
understand why this is needed). System is gentoo linux and gcc is 3.4.4:

extern "C"
{
extern GOPluginModuleDepend const go_plugin_depends [] = {
    { "goffice", GOFFICE_API_VERSION }
};
extern GOPluginModuleHeader const go_plugin_header =
	{ GOFFICE_MODULE_PLUGIN_MAGIC_NUMBER, G_N_ELEMENTS (go_plugin_depends) };
}
Comment 10 Ivan Wong 2005-11-07 16:39:06 UTC
jody: what do you think about this one? should be quite similar to #306740
Comment 11 Olav Vitters 2005-12-20 17:24:55 UTC
Created attachment 56215 [details] [review]
Just testing.
Comment 12 Olav Vitters 2005-12-20 18:11:15 UTC
Created attachment 56220 [details] [review]
efew
Comment 13 Olav Vitters 2005-12-20 18:12:02 UTC
Created attachment 56221 [details] [review]
revised

Updated to the HEAD
Comment 14 Ivan Wong 2006-01-23 18:20:06 UTC
Created attachment 57957 [details] [review]
revised

Updated the patch to HEAD.

Could anybody please review this patch?
Comment 15 Ivan Wong 2006-02-17 16:51:47 UTC
Committed.