[PATCH libICE] Enable visibility annotations

[Emil Velikov]

On 10 May 2016 at 13:00, Yury Gribov <y.gribov at samsung.com> wrote:
> On 05/06/2016 07:25 PM, Alan Coopersmith wrote:
>>
>> On 05/ 6/16 04:18 AM, Yury Gribov wrote:
>>>
>>> Unfortunately I don't see any good approach. We could move definitions of
>>> _X_EXPORT and friends to configure.ac but this would need to be done
>>> for all X11 packages (not just libICE) which just does not scale.
>>
>>
>> And then you'd require any package that #includes an X11 header to use
>> autoconf and add those to configure.ac and that really doesn't scale.
>
>
> There is a way around this: you can strip internal defines from public
> headers and replace it with e.g. extern. This is how visibility was enabled
> for acl and libattr some time ago
> (http://lists.nongnu.org/archive/html/acl-devel/2016-02/msg00020.html).
>
If you're suggesting that we should remove all the internal prototypes
(declarations) from the exported headers, yes that would be great. If
you're thinking about defining _X_EXPORT via extern that won't work,
as it will break way too many applications that set
-fvisibility=hidden or alike. Or perhaps you're thinking about nuking
the macro all together ? Sadly that won't work either, as it will
break way too many other (sub)projects that depend on it.

On the WIN32 (original?) topic
 - _X_EXPORT implemented via __declspec dllimport/dllexport
Works fine on mingw-w64 - and the resulting binary has only the
annotated symbols as exported.
 - fvisibility=hidden
Does nothing on mingw64, although it does not cause any warning/errors.

So in general something like the following should work
Note: mingw and cygwin will need to drop fvisibility and/or alike and
set the BUILDING_DLL define

--- a/Xfuncproto.h.orig
+++ b/Xfuncproto.h.new
@@ -89,8 +89,15 @@ in this Software without prior written
authorization from The Open Group.
#endif /* GNUC >= 4 */

/* Added in X11R6.9, so available in any version of modular xproto */
-#if (__has_attribute(visibility) || (defined(__GNUC__) && (__GNUC__ >= 4))) \
-    && !defined(__CYGWIN__) && !defined(__MINGW32__)
+#if defined(_WIN32) || defined(__CYGWIN__) && defined(__MINGW32__)
+# ifdef BUILDING_DLL // should we prefix this with _X_ ?
+#  define _X_EXPORT __declspec(dllexport)
+# else
+#  define _X_EXPORT __declspec(dllimport)
+# endif
+# define _X_HIDDEN
+# define _X_INTERNAL
+#elif __has_attribute(visibility) || (defined(__GNUC__) && (__GNUC__ >= 4))
# define _X_EXPORT      __attribute__((visibility("default")))
# define _X_HIDDEN      __attribute__((visibility("hidden")))
# define _X_INTERNAL    __attribute__((visibility("internal")))


Jon, have I lost the plot with any of the above ? Please don't be shy
to say "yes you have" ;-)
Can you give it a quick test on your end ?

Thanks
Emil