Currently, the drm_fourcc.h header depends on drm.h for __u32 and __u64. At the same time drm.h pulls a lot of unneeded symbols.
Add new guard DRM_FOURCC_STANDALONE, which when set will use local declaration of said symbols.
When used on linux - we pull linux/types.h which is used either way. On other platforms, BSDs et al, we need a couple of typedefs.
Since those can trigger a warning in some corner-cases*, add some GCC magic to silence them. Note that incorrect type redefinitions will still be flagged, and the GCC pragma is ignored by other compilers.
*Corner-case: If one sets DRM_FOURCC_STANDALONE and compiles with C99 or earlier while also using -pedantic _and_ the header lives outside of the standard /usr/include (like BSDs normally do).
v2: - Add corner-case handling, based on popular demand.
Cc: James Park james.park@lagfreegames.com Cc: Pekka Paalanen pekka.paalanen@collabora.com Cc: Simon Ser contact@emersion.fr Signed-off-by: Emil Velikov emil.l.velikov@gmail.com --- include/uapi/drm/drm.h | 10 ++++++++++ include/uapi/drm/drm_fourcc.h | 29 +++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+)
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 808b48a93330..cd78950e05ce 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -53,6 +53,15 @@ typedef unsigned int drm_handle_t; #include <stdint.h> #include <sys/ioccom.h> #include <sys/types.h> + +/* + * When using C99 -pedantic the typedefs will trigger a warning. + * If the header is considered a system one (-isystem) those will be + * ignored, yet on the target platforms BSDs, et al - the headers live + * in a non-system location. + */ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wpedantic" typedef int8_t __s8; typedef uint8_t __u8; typedef int16_t __s16; @@ -63,6 +72,7 @@ typedef int64_t __s64; typedef uint64_t __u64; typedef size_t __kernel_size_t; typedef unsigned long drm_handle_t; +#pragma GCC diagnostic pop
#endif
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h index 6f0628eb13a6..84a1f96cc4ef 100644 --- a/include/uapi/drm/drm_fourcc.h +++ b/include/uapi/drm/drm_fourcc.h @@ -24,7 +24,36 @@ #ifndef DRM_FOURCC_H #define DRM_FOURCC_H
+/* + * Define DRM_FOURCC_STANDALONE you're interested only FOURCC and do not want + * to pull drm.h into your application. + */ +#ifdef DRM_FOURCC_STANDALONE +#if defined(__linux__) + +#include <linux/types.h> + +#else /* One of the BSDs */ + +#include <stdint.h> + +/* + * When using C99 -pedantic the typedefs will trigger a warning. + * If the header is considered a system one (-isystem) those will be + * ignored, yet on the target platforms BSDs, et al - the headers live + * in a non-system location. + */ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wpedantic" +typedef uint32_t __u32; +typedef uint64_t __u64; +#pragma GCC diagnostic pop + +#endif /* __linux __ */ + +#else #include "drm.h" +#endif /* DRM_FOURCC_STANDALONE */
#if defined(__cplusplus) extern "C" {
On Thu, Feb 4, 2021 at 3:37 AM Emil Velikov emil.l.velikov@gmail.com wrote:
Currently, the drm_fourcc.h header depends on drm.h for __u32 and __u64. At the same time drm.h pulls a lot of unneeded symbols.
Add new guard DRM_FOURCC_STANDALONE, which when set will use local declaration of said symbols.
When used on linux - we pull linux/types.h which is used either way. On other platforms, BSDs et al, we need a couple of typedefs.
Since those can trigger a warning in some corner-cases*, add some GCC magic to silence them. Note that incorrect type redefinitions will still be flagged, and the GCC pragma is ignored by other compilers.
*Corner-case: If one sets DRM_FOURCC_STANDALONE and compiles with C99 or earlier while also using -pedantic _and_ the header lives outside of the standard /usr/include (like BSDs normally do).
v2:
- Add corner-case handling, based on popular demand.
Cc: James Park james.park@lagfreegames.com Cc: Pekka Paalanen pekka.paalanen@collabora.com Cc: Simon Ser contact@emersion.fr Signed-off-by: Emil Velikov emil.l.velikov@gmail.com
include/uapi/drm/drm.h | 10 ++++++++++ include/uapi/drm/drm_fourcc.h | 29 +++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+)
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 808b48a93330..cd78950e05ce 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -53,6 +53,15 @@ typedef unsigned int drm_handle_t; #include <stdint.h> #include <sys/ioccom.h> #include <sys/types.h>
+/*
- When using C99 -pedantic the typedefs will trigger a warning.
- If the header is considered a system one (-isystem) those will be
- ignored, yet on the target platforms BSDs, et al - the headers live
- in a non-system location.
- */
+#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wpedantic" typedef int8_t __s8; typedef uint8_t __u8; typedef int16_t __s16; @@ -63,6 +72,7 @@ typedef int64_t __s64; typedef uint64_t __u64; typedef size_t __kernel_size_t; typedef unsigned long drm_handle_t; +#pragma GCC diagnostic pop
#endif
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h index 6f0628eb13a6..84a1f96cc4ef 100644 --- a/include/uapi/drm/drm_fourcc.h +++ b/include/uapi/drm/drm_fourcc.h @@ -24,7 +24,36 @@ #ifndef DRM_FOURCC_H #define DRM_FOURCC_H
+/*
- Define DRM_FOURCC_STANDALONE you're interested only FOURCC and do not want
- to pull drm.h into your application.
- */
+#ifdef DRM_FOURCC_STANDALONE +#if defined(__linux__)
+#include <linux/types.h>
+#else /* One of the BSDs */
+#include <stdint.h>
+/*
- When using C99 -pedantic the typedefs will trigger a warning.
- If the header is considered a system one (-isystem) those will be
- ignored, yet on the target platforms BSDs, et al - the headers live
- in a non-system location.
- */
+#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wpedantic" +typedef uint32_t __u32; +typedef uint64_t __u64; +#pragma GCC diagnostic pop
+#endif /* __linux __ */
+#else #include "drm.h" +#endif /* DRM_FOURCC_STANDALONE */
#if defined(__cplusplus) extern "C" { -- 2.30.0
I remember reading GCC diagnostic push/pop requires a recent enough compiler version to be supported, which is pretty old, but I don't know how old is old enough for Linux headers: https://github.com/protocolbuffers/protobuf/issues/4156
Testing snippets in godbolt, I think the pragmas need to be wrapped. MSVC says:
warning C4068: unknown pragma 'GCC'
Also, Clang seems to want -Wtypedef-redefinition, not -Wpedantic. GCC complains it doesn't know what -Wtypedef-redefinition is, so that would also need to be wrapped.
On Thu, Feb 4, 2021 at 9:37 AM James Park james.park@lagfreegames.com wrote:
On Thu, Feb 4, 2021 at 3:37 AM Emil Velikov emil.l.velikov@gmail.com wrote:
Currently, the drm_fourcc.h header depends on drm.h for __u32 and __u64. At the same time drm.h pulls a lot of unneeded symbols.
Add new guard DRM_FOURCC_STANDALONE, which when set will use local declaration of said symbols.
When used on linux - we pull linux/types.h which is used either way. On other platforms, BSDs et al, we need a couple of typedefs.
Since those can trigger a warning in some corner-cases*, add some GCC magic to silence them. Note that incorrect type redefinitions will still be flagged, and the GCC pragma is ignored by other compilers.
*Corner-case: If one sets DRM_FOURCC_STANDALONE and compiles with C99 or earlier while also using -pedantic _and_ the header lives outside of the standard /usr/include (like BSDs normally do).
v2:
- Add corner-case handling, based on popular demand.
Cc: James Park james.park@lagfreegames.com Cc: Pekka Paalanen pekka.paalanen@collabora.com Cc: Simon Ser contact@emersion.fr Signed-off-by: Emil Velikov emil.l.velikov@gmail.com
include/uapi/drm/drm.h | 10 ++++++++++ include/uapi/drm/drm_fourcc.h | 29 +++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+)
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 808b48a93330..cd78950e05ce 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -53,6 +53,15 @@ typedef unsigned int drm_handle_t; #include <stdint.h> #include <sys/ioccom.h> #include <sys/types.h>
+/*
- When using C99 -pedantic the typedefs will trigger a warning.
- If the header is considered a system one (-isystem) those will be
- ignored, yet on the target platforms BSDs, et al - the headers live
- in a non-system location.
- */
+#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wpedantic" typedef int8_t __s8; typedef uint8_t __u8; typedef int16_t __s16; @@ -63,6 +72,7 @@ typedef int64_t __s64; typedef uint64_t __u64; typedef size_t __kernel_size_t; typedef unsigned long drm_handle_t; +#pragma GCC diagnostic pop
#endif
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h index 6f0628eb13a6..84a1f96cc4ef 100644 --- a/include/uapi/drm/drm_fourcc.h +++ b/include/uapi/drm/drm_fourcc.h @@ -24,7 +24,36 @@ #ifndef DRM_FOURCC_H #define DRM_FOURCC_H
+/*
- Define DRM_FOURCC_STANDALONE you're interested only FOURCC and do not want
- to pull drm.h into your application.
- */
+#ifdef DRM_FOURCC_STANDALONE +#if defined(__linux__)
+#include <linux/types.h>
+#else /* One of the BSDs */
+#include <stdint.h>
+/*
- When using C99 -pedantic the typedefs will trigger a warning.
- If the header is considered a system one (-isystem) those will be
- ignored, yet on the target platforms BSDs, et al - the headers live
- in a non-system location.
- */
+#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wpedantic" +typedef uint32_t __u32; +typedef uint64_t __u64; +#pragma GCC diagnostic pop
+#endif /* __linux __ */
+#else #include "drm.h" +#endif /* DRM_FOURCC_STANDALONE */
#if defined(__cplusplus) extern "C" { -- 2.30.0
I remember reading GCC diagnostic push/pop requires a recent enough compiler version to be supported, which is pretty old, but I don't know how old is old enough for Linux headers: https://github.com/protocolbuffers/protobuf/issues/4156
Testing snippets in godbolt, I think the pragmas need to be wrapped. MSVC says:
warning C4068: unknown pragma 'GCC'
Also, Clang seems to want -Wtypedef-redefinition, not -Wpedantic. GCC complains it doesn't know what -Wtypedef-redefinition is, so that would also need to be wrapped.
If we're already conceding copy/paste, then how about inlining my previous header?
#ifndef DRM_BASIC_TYPES_DEFINED #define DRM_BASIC_TYPES_DEFINED // Sync modifications between drm.h and drm_fourcc.h ... #endif
No compiler games. Valid on all flavors of C (I think).
On Thu, 4 Feb 2021 at 18:17, James Park james.park@lagfreegames.com wrote:
On Thu, Feb 4, 2021 at 9:37 AM James Park james.park@lagfreegames.com wrote:
On Thu, Feb 4, 2021 at 3:37 AM Emil Velikov emil.l.velikov@gmail.com wrote:
Currently, the drm_fourcc.h header depends on drm.h for __u32 and __u64. At the same time drm.h pulls a lot of unneeded symbols.
Add new guard DRM_FOURCC_STANDALONE, which when set will use local declaration of said symbols.
When used on linux - we pull linux/types.h which is used either way. On other platforms, BSDs et al, we need a couple of typedefs.
Since those can trigger a warning in some corner-cases*, add some GCC magic to silence them. Note that incorrect type redefinitions will still be flagged, and the GCC pragma is ignored by other compilers.
*Corner-case: If one sets DRM_FOURCC_STANDALONE and compiles with C99 or earlier while also using -pedantic _and_ the header lives outside of the standard /usr/include (like BSDs normally do).
v2:
- Add corner-case handling, based on popular demand.
Cc: James Park james.park@lagfreegames.com Cc: Pekka Paalanen pekka.paalanen@collabora.com Cc: Simon Ser contact@emersion.fr Signed-off-by: Emil Velikov emil.l.velikov@gmail.com
include/uapi/drm/drm.h | 10 ++++++++++ include/uapi/drm/drm_fourcc.h | 29 +++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+)
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 808b48a93330..cd78950e05ce 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -53,6 +53,15 @@ typedef unsigned int drm_handle_t; #include <stdint.h> #include <sys/ioccom.h> #include <sys/types.h>
+/*
- When using C99 -pedantic the typedefs will trigger a warning.
- If the header is considered a system one (-isystem) those will be
- ignored, yet on the target platforms BSDs, et al - the headers live
- in a non-system location.
- */
+#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wpedantic" typedef int8_t __s8; typedef uint8_t __u8; typedef int16_t __s16; @@ -63,6 +72,7 @@ typedef int64_t __s64; typedef uint64_t __u64; typedef size_t __kernel_size_t; typedef unsigned long drm_handle_t; +#pragma GCC diagnostic pop
#endif
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h index 6f0628eb13a6..84a1f96cc4ef 100644 --- a/include/uapi/drm/drm_fourcc.h +++ b/include/uapi/drm/drm_fourcc.h @@ -24,7 +24,36 @@ #ifndef DRM_FOURCC_H #define DRM_FOURCC_H
+/*
- Define DRM_FOURCC_STANDALONE you're interested only FOURCC and do not want
- to pull drm.h into your application.
- */
+#ifdef DRM_FOURCC_STANDALONE +#if defined(__linux__)
+#include <linux/types.h>
+#else /* One of the BSDs */
+#include <stdint.h>
+/*
- When using C99 -pedantic the typedefs will trigger a warning.
- If the header is considered a system one (-isystem) those will be
- ignored, yet on the target platforms BSDs, et al - the headers live
- in a non-system location.
- */
+#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wpedantic" +typedef uint32_t __u32; +typedef uint64_t __u64; +#pragma GCC diagnostic pop
+#endif /* __linux __ */
+#else #include "drm.h" +#endif /* DRM_FOURCC_STANDALONE */
#if defined(__cplusplus) extern "C" { -- 2.30.0
I remember reading GCC diagnostic push/pop requires a recent enough compiler version to be supported, which is pretty old, but I don't know how old is old enough for Linux headers: https://github.com/protocolbuffers/protobuf/issues/4156
Testing snippets in godbolt, I think the pragmas need to be wrapped. MSVC says:
warning C4068: unknown pragma 'GCC'
/me shakes fist at MSVC - why are you being so silly
Also, Clang seems to want -Wtypedef-redefinition, not -Wpedantic. GCC complains it doesn't know what -Wtypedef-redefinition is, so that would also need to be wrapped.
Clang seemed fine here. Must have used a different version or something.
If we're already conceding copy/paste, then how about inlining my previous header?
#ifndef DRM_BASIC_TYPES_DEFINED #define DRM_BASIC_TYPES_DEFINED // Sync modifications between drm.h and drm_fourcc.h ... #endif
No compiler games. Valid on all flavors of C (I think).
Hmm cannot find any patch mentioning DRM_BASIC_TYPES_DEFINED - perhaps you did one in the mesa MR?
Either way, if the proposal is to have the include/typedefs guarded as above - sure, that works. Please add the guard in both drm.h and drm_fourcc.h The comment seems slightly confusing, but that's nitpicking.
Do send a patch, unless someone shouts against it, I'll be happy to push it and churn the whole copy to drm/mesa game.
Thanks Emil
On Thu, Feb 4, 2021 at 1:07 PM Emil Velikov emil.l.velikov@gmail.com wrote:
On Thu, 4 Feb 2021 at 18:17, James Park james.park@lagfreegames.com wrote:
On Thu, Feb 4, 2021 at 9:37 AM James Park james.park@lagfreegames.com wrote:
On Thu, Feb 4, 2021 at 3:37 AM Emil Velikov emil.l.velikov@gmail.com wrote:
Currently, the drm_fourcc.h header depends on drm.h for __u32 and __u64. At the same time drm.h pulls a lot of unneeded symbols.
Add new guard DRM_FOURCC_STANDALONE, which when set will use local declaration of said symbols.
When used on linux - we pull linux/types.h which is used either way. On other platforms, BSDs et al, we need a couple of typedefs.
Since those can trigger a warning in some corner-cases*, add some GCC magic to silence them. Note that incorrect type redefinitions will still be flagged, and the GCC pragma is ignored by other compilers.
*Corner-case: If one sets DRM_FOURCC_STANDALONE and compiles with C99 or earlier while also using -pedantic _and_ the header lives outside of the standard /usr/include (like BSDs normally do).
v2:
- Add corner-case handling, based on popular demand.
Cc: James Park james.park@lagfreegames.com Cc: Pekka Paalanen pekka.paalanen@collabora.com Cc: Simon Ser contact@emersion.fr Signed-off-by: Emil Velikov emil.l.velikov@gmail.com
include/uapi/drm/drm.h | 10 ++++++++++ include/uapi/drm/drm_fourcc.h | 29 +++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+)
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 808b48a93330..cd78950e05ce 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -53,6 +53,15 @@ typedef unsigned int drm_handle_t; #include <stdint.h> #include <sys/ioccom.h> #include <sys/types.h>
+/*
- When using C99 -pedantic the typedefs will trigger a warning.
- If the header is considered a system one (-isystem) those will be
- ignored, yet on the target platforms BSDs, et al - the headers live
- in a non-system location.
- */
+#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wpedantic" typedef int8_t __s8; typedef uint8_t __u8; typedef int16_t __s16; @@ -63,6 +72,7 @@ typedef int64_t __s64; typedef uint64_t __u64; typedef size_t __kernel_size_t; typedef unsigned long drm_handle_t; +#pragma GCC diagnostic pop
#endif
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h index 6f0628eb13a6..84a1f96cc4ef 100644 --- a/include/uapi/drm/drm_fourcc.h +++ b/include/uapi/drm/drm_fourcc.h @@ -24,7 +24,36 @@ #ifndef DRM_FOURCC_H #define DRM_FOURCC_H
+/*
- Define DRM_FOURCC_STANDALONE you're interested only FOURCC and do not want
- to pull drm.h into your application.
- */
+#ifdef DRM_FOURCC_STANDALONE +#if defined(__linux__)
+#include <linux/types.h>
+#else /* One of the BSDs */
+#include <stdint.h>
+/*
- When using C99 -pedantic the typedefs will trigger a warning.
- If the header is considered a system one (-isystem) those will be
- ignored, yet on the target platforms BSDs, et al - the headers live
- in a non-system location.
- */
+#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wpedantic" +typedef uint32_t __u32; +typedef uint64_t __u64; +#pragma GCC diagnostic pop
+#endif /* __linux __ */
+#else #include "drm.h" +#endif /* DRM_FOURCC_STANDALONE */
#if defined(__cplusplus) extern "C" { -- 2.30.0
I remember reading GCC diagnostic push/pop requires a recent enough compiler version to be supported, which is pretty old, but I don't know how old is old enough for Linux headers: https://github.com/protocolbuffers/protobuf/issues/4156
Testing snippets in godbolt, I think the pragmas need to be wrapped. MSVC says:
warning C4068: unknown pragma 'GCC'
/me shakes fist at MSVC - why are you being so silly
Also, Clang seems to want -Wtypedef-redefinition, not -Wpedantic. GCC complains it doesn't know what -Wtypedef-redefinition is, so that would also need to be wrapped.
Clang seemed fine here. Must have used a different version or something.
If we're already conceding copy/paste, then how about inlining my previous header?
#ifndef DRM_BASIC_TYPES_DEFINED #define DRM_BASIC_TYPES_DEFINED // Sync modifications between drm.h and drm_fourcc.h ... #endif
No compiler games. Valid on all flavors of C (I think).
Hmm cannot find any patch mentioning DRM_BASIC_TYPES_DEFINED - perhaps you did one in the mesa MR?
Either way, if the proposal is to have the include/typedefs guarded as above - sure, that works. Please add the guard in both drm.h and drm_fourcc.h The comment seems slightly confusing, but that's nitpicking.
Do send a patch, unless someone shouts against it, I'll be happy to push it and churn the whole copy to drm/mesa game.
Thanks Emil
No, I haven't pushed anything with DRM_BASIC_TYPES_DEFINED yet. I was just illustrating since _DRM_BASIC_TYPES_H_ would not be a great choice for a name in this case.
I can try to come up with a better comment.
I'll send a patch tonight (USA time). I have to remember how to do the git-send-email dance.
Thanks, James
On Thursday, February 4th, 2021 at 10:15 PM, James Park james.park@lagfreegames.com wrote:
No, I haven't pushed anything with DRM_BASIC_TYPES_DEFINED yet. I was just illustrating since DRM_BASIC_TYPES_H would not be a great choice for a name in this case.
This approach looks fine to me too. I'd R-b it.
Thanks for your patience.
I'll send a patch tonight (USA time). I have to remember how to do the git-send-email dance.
On Thu, Feb 4, 2021 at 1:07 PM Emil Velikov emil.l.velikov@gmail.com wrote:
Do send a patch, unless someone shouts against it, I'll be happy to push it and churn the whole copy to drm/mesa game.
Thanks Emil
Hi Emil,
Were you still planning on pushing the patch? I sent a v2 on Feb 8.
Thanks, James
On Fri, Feb 26, 2021 at 12:08 AM James Park james.park@lagfreegames.com wrote:
On Thu, Feb 4, 2021 at 1:07 PM Emil Velikov emil.l.velikov@gmail.com wrote:
Do send a patch, unless someone shouts against it, I'll be happy to push it and churn the whole copy to drm/mesa game.
Thanks Emil
Hi Emil,
Were you still planning on pushing the patch? I sent a v2 on Feb 8.
Thanks, James
Looping in Jason Ekstrand who wants this patch for his own purposes. To answer his question in the other email thread, I stopped pushing once I decided to just copy/paste the necessary definitions for my own use case to keep that ball rolling.
- James
- James
dri-devel@lists.freedesktop.org