diff mbox

[v2,libdrm,1/7] configure: Support symbol visibility when available

Message ID 1397043630-13972-2-git-send-email-thierry.reding@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Thierry Reding April 9, 2014, 11:40 a.m. UTC 
From: Thierry Reding <treding@nvidia.com>

Checks whether or not the compiler supports the -fvisibility option. If
so it sets the VISIBILITY_CFLAGS variable which can be added to the per
directory AM_CFLAGS where appropriate.

By default all symbols will be hidden via the VISIBILITY_CFLAGS. The
drm_public macro can be used to mark symbols that should be exported.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 configure.ac | 20 ++++++++++++++++++++
 libdrm.h     | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+)
 create mode 100644 libdrm.h

Comments

Erik Faye-Lund April 10, 2014, 5:15 p.m. UTC | #1 
On Wed, Apr 9, 2014 at 1:40 PM, Thierry Reding <thierry.reding@gmail.com> wrote:
> diff --git a/libdrm.h b/libdrm.h
> new file mode 100644
> index 000000000000..23926e6f6741
> --- /dev/null
> +++ b/libdrm.h
> @@ -0,0 +1,34 @@
> +/*
> + * Copyright © 2014 NVIDIA Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +#ifndef LIBDRM_LIBDRM_H
> +#define LIBDRM_LIBDRM_H

LIBDRM_LIBDRM_H sounds a bit clunky to me. Why LIBDRM twice? The other
headers don't seem to prefix LIBDRM_ to their header-guards. In fact,
many of them don't even have header-guards.

Also, does these macro really warrant making a top-level, generically
named header?
Thierry Reding May 2, 2014, 2:12 p.m. UTC | #2 
On Thu, Apr 10, 2014 at 07:15:14PM +0200, Erik Faye-Lund wrote:
> On Wed, Apr 9, 2014 at 1:40 PM, Thierry Reding <thierry.reding@gmail.com> wrote:
> > diff --git a/libdrm.h b/libdrm.h
> > new file mode 100644
> > index 000000000000..23926e6f6741
> > --- /dev/null
> > +++ b/libdrm.h
> > @@ -0,0 +1,34 @@
> > +/*
> > + * Copyright © 2014 NVIDIA Corporation
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a
> > + * copy of this software and associated documentation files (the "Software"),
> > + * to deal in the Software without restriction, including without limitation
> > + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > + * and/or sell copies of the Software, and to permit persons to whom the
> > + * Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice shall be included in
> > + * all copies or substantial portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> > + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> > + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> > + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> > + * OTHER DEALINGS IN THE SOFTWARE.
> > + */
> > +
> > +#ifndef LIBDRM_LIBDRM_H
> > +#define LIBDRM_LIBDRM_H
> 
> LIBDRM_LIBDRM_H sounds a bit clunky to me. Why LIBDRM twice? The other
> headers don't seem to prefix LIBDRM_ to their header-guards. In fact,
> many of them don't even have header-guards.

This was with the intention of marking it as an internal header file. So
the LIBDRM_ prefix could be used consistently for all files that are not
installed. xf86atomic.h uses that prefix as well.

> Also, does these macro really warrant making a top-level, generically
> named header?

There isn't really another header file where this would fit. Others are
either installed (and therefore shouldn't be exposing these macros) or
have a very specific purpose (xf86atomic.h).

Thierry
Erik Faye-Lund May 2, 2014, 2:59 p.m. UTC | #3 
On Fri, May 2, 2014 at 4:12 PM, Thierry Reding <thierry.reding@gmail.com> wrote:
> On Thu, Apr 10, 2014 at 07:15:14PM +0200, Erik Faye-Lund wrote:
>> On Wed, Apr 9, 2014 at 1:40 PM, Thierry Reding <thierry.reding@gmail.com> wrote:
>> > diff --git a/libdrm.h b/libdrm.h
>> > new file mode 100644
>> > index 000000000000..23926e6f6741
>> > --- /dev/null
>> > +++ b/libdrm.h
>> > @@ -0,0 +1,34 @@
>> > +/*
>> > + * Copyright © 2014 NVIDIA Corporation
>> > + *
>> > + * Permission is hereby granted, free of charge, to any person obtaining a
>> > + * copy of this software and associated documentation files (the "Software"),
>> > + * to deal in the Software without restriction, including without limitation
>> > + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> > + * and/or sell copies of the Software, and to permit persons to whom the
>> > + * Software is furnished to do so, subject to the following conditions:
>> > + *
>> > + * The above copyright notice and this permission notice shall be included in
>> > + * all copies or substantial portions of the Software.
>> > + *
>> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
>> > + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
>> > + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>> > + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>> > + * OTHER DEALINGS IN THE SOFTWARE.
>> > + */
>> > +
>> > +#ifndef LIBDRM_LIBDRM_H
>> > +#define LIBDRM_LIBDRM_H
>>
>> LIBDRM_LIBDRM_H sounds a bit clunky to me. Why LIBDRM twice? The other
>> headers don't seem to prefix LIBDRM_ to their header-guards. In fact,
>> many of them don't even have header-guards.
>
> This was with the intention of marking it as an internal header file. So
> the LIBDRM_ prefix could be used consistently for all files that are not
> installed. xf86atomic.h uses that prefix as well.

If you look at the history of xf86atomic.h, it seems this strange
header-guard is the result of a slightly careless replace. It used to
be called intel_atomics.h, and have INTEL_ATOMICS_H as the
header-guard. So I wouldn't lake that set too much of a precedence.

>> Also, does these macro really warrant making a top-level, generically
>> named header?
>
> There isn't really another header file where this would fit. Others are
> either installed (and therefore shouldn't be exposing these macros) or
> have a very specific purpose (xf86atomic.h).

I guess this is a matter of taste, and it would be great with some
input from the other libdrm-people on this. I don't care too much
either way...
diff mbox

Patch

diff --git a/configure.ac b/configure.ac
index b7eef9673a20..22b45cc7560d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -366,6 +366,26 @@  AC_ARG_WITH([kernel-source],
 	    [kernel_source="$with_kernel_source"])
 AC_SUBST(kernel_source)
 
+dnl Add flags for gcc and g++
+if test "x$GCC" = xyes; then
+    # Enable -fvisibility=hidden if using a gcc that supports it
+    save_CFLAGS="$CFLAGS"
+    AC_MSG_CHECKING([whether $CC supports -fvisibility=hidden])
+    VISIBILITY_CFLAGS="-fvisibility=hidden"
+    CFLAGS="$CFLAGS $VISIBILITY_CFLAGS"
+    AC_LINK_IFELSE([AC_LANG_PROGRAM()], AC_MSG_RESULT([yes]),
+                   [VISIBILITY_CFLAGS=""; AC_MSG_RESULT([no])]);
+
+    # Restore CFLAGS; VISIBILITY_CFLAGS are added to it where needed.
+    CFLAGS=$save_CFLAGS
+
+    if test "x$VISIBILITY_CFLAGS" != x; then
+        AC_DEFINE(HAVE_VISIBILITY, 1, [Compiler has -fvisibility support])
+    fi
+
+    AC_SUBST([VISIBILITY_CFLAGS])
+fi
+
 AC_SUBST(WARN_CFLAGS)
 AC_CONFIG_FILES([
 	Makefile
diff --git a/libdrm.h b/libdrm.h
new file mode 100644
index 000000000000..23926e6f6741
--- /dev/null
+++ b/libdrm.h
@@ -0,0 +1,34 @@ 
+/*
+ * Copyright © 2014 NVIDIA Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#ifndef LIBDRM_LIBDRM_H
+#define LIBDRM_LIBDRM_H
+
+#if defined(HAVE_VISIBILITY)
+#  define drm_private __attribute__((visibility("hidden")))
+#  define drm_public __attribute__((visibility("default")))
+#else
+#  define drm_private
+#  define drm_public
+#endif
+
+#endif