xf86drmMode.h: inline -> __inline for use with gcc -std=c89 -pedantic
diff mbox

Message ID 1427245262-28865-1-git-send-email-djkurtz@chromium.org
State New, archived
Headers show

Commit Message

Daniel Kurtz March 25, 2015, 1:01 a.m. UTC
Unfortunately, there are some users of libdrm installed headers that like
to be built with -std=c89 -pedantic, which does not like "inline".

However, __inline works.

Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
---
 xf86drmMode.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Emil Velikov March 26, 2015, 3:12 p.m. UTC | #1
Hi Daniel,
On 25/03/15 01:01, Daniel Kurtz wrote:
> Unfortunately, there are some users of libdrm installed headers that like
> to be built with -std=c89 -pedantic, which does not like "inline".
> 
> However, __inline works.
> 
> Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>

> ---
>  xf86drmMode.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xf86drmMode.h b/xf86drmMode.h
> index 856a6bb..2d30184 100644
> --- a/xf86drmMode.h
> +++ b/xf86drmMode.h
> @@ -240,7 +240,7 @@ typedef struct _drmModeProperty {
>  	uint32_t *blob_ids; /* store the blob IDs */
>  } drmModePropertyRes, *drmModePropertyPtr;
>  
> -static inline int drm_property_type_is(drmModePropertyPtr property,
> +static __inline int drm_property_type_is(drmModePropertyPtr property,

Can you share the name of those users ?

From a quick look clang and gcc are ok with this change, although some
versions of the Sun compiler are not. Considering that most programs try
to use c99 and later, might I suggest that you add a trivial wrapper in
your program ? Something along the lines of

#ifndef inline
#define inline __inline
#endif


-Emil
Daniel Kurtz March 26, 2015, 3:38 p.m. UTC | #2
Hi Emil,

On Thu, Mar 26, 2015 at 11:12 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>
> Hi Daniel,
> On 25/03/15 01:01, Daniel Kurtz wrote:
> > Unfortunately, there are some users of libdrm installed headers that like
> > to be built with -std=c89 -pedantic, which does not like "inline".
> >
> > However, __inline works.
> >
> > Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
>
> > ---
> >  xf86drmMode.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/xf86drmMode.h b/xf86drmMode.h
> > index 856a6bb..2d30184 100644
> > --- a/xf86drmMode.h
> > +++ b/xf86drmMode.h
> > @@ -240,7 +240,7 @@ typedef struct _drmModeProperty {
> >       uint32_t *blob_ids; /* store the blob IDs */
> >  } drmModePropertyRes, *drmModePropertyPtr;
> >
> > -static inline int drm_property_type_is(drmModePropertyPtr property,
> > +static __inline int drm_property_type_is(drmModePropertyPtr property,
>
> Can you share the name of those users ?
>
> From a quick look clang and gcc are ok with this change, although some
> versions of the Sun compiler are not. Considering that most programs try
> to use c99 and later, might I suggest that you add a trivial wrapper in
> your program ? Something along the lines of
>
> #ifndef inline
> #define inline __inline
> #endif

It isn't a program, its a third party EGL/GLES driver, so, no I can't
really do this easily.

Alternatively can we:
 (1) move the wrapper to xf86drmMode.h itself, or
 (2) move this inline helper function out of xf86drmMode.h and into
the two libdrm tests that use it (or a shared test helper .h [0])
 (3) remove the inline and make drm_property_type_is a non-inline
function in xf86drmMode.c

[0] I think Thierry already has a patch set that rearranges the tests
to use some shared headers.  Perhaps this will work well once his set
lands.

-Dan

>
>
> -Emil
>
Emil Velikov March 26, 2015, 4:21 p.m. UTC | #3
On 26/03/15 15:38, Daniel Kurtz wrote:
> Hi Emil,
> 
> On Thu, Mar 26, 2015 at 11:12 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>>
>> Hi Daniel,
>> On 25/03/15 01:01, Daniel Kurtz wrote:
>>> Unfortunately, there are some users of libdrm installed headers that like
>>> to be built with -std=c89 -pedantic, which does not like "inline".
>>>
>>> However, __inline works.
>>>
>>> Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
>>
>>> ---
>>>  xf86drmMode.h | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/xf86drmMode.h b/xf86drmMode.h
>>> index 856a6bb..2d30184 100644
>>> --- a/xf86drmMode.h
>>> +++ b/xf86drmMode.h
>>> @@ -240,7 +240,7 @@ typedef struct _drmModeProperty {
>>>       uint32_t *blob_ids; /* store the blob IDs */
>>>  } drmModePropertyRes, *drmModePropertyPtr;
>>>
>>> -static inline int drm_property_type_is(drmModePropertyPtr property,
>>> +static __inline int drm_property_type_is(drmModePropertyPtr property,
>>
>> Can you share the name of those users ?
>>
>> From a quick look clang and gcc are ok with this change, although some
>> versions of the Sun compiler are not. Considering that most programs try
>> to use c99 and later, might I suggest that you add a trivial wrapper in
>> your program ? Something along the lines of
>>
>> #ifndef inline
>> #define inline __inline
>> #endif
> 
> It isn't a program, its a third party EGL/GLES driver, so, no I can't
> really do this easily.
> 
I see... due to various agreements you're not allowed to make any
changes. Be that to the code or the build (-Dinline...).

> Alternatively can we:
>  (1) move the wrapper to xf86drmMode.h itself, or
>  (2) move this inline helper function out of xf86drmMode.h and into
> the two libdrm tests that use it (or a shared test helper .h [0])
>  (3) remove the inline and make drm_property_type_is a non-inline
> function in xf86drmMode.c
> 
> [0] I think Thierry already has a patch set that rearranges the tests
> to use some shared headers.  Perhaps this will work well once his set
> lands.
> 
Fwiw I would opt for 1 or 3 (leaning towards 1), as 2 might lead to:
 - Everyone coming up with their own "helper", or
 - Ignoring DRM_MODE_PROP_EXTENDED_TYPE checking
drmModePropertyPtr::flags, thus causing all sorts of chaos.


Randy, Niveditha, Stuart,

Does the compiler(s) used to build libdrm and friends support __inline ?

Thanks,
Emil

P.S. [0] Seems to be missing but I do recall the series :)
randyf@sibernet.com March 26, 2015, 4:57 p.m. UTC | #4
>
>> Alternatively can we:
>>  (1) move the wrapper to xf86drmMode.h itself, or
>>  (2) move this inline helper function out of xf86drmMode.h and into
>> the two libdrm tests that use it (or a shared test helper .h [0])
>>  (3) remove the inline and make drm_property_type_is a non-inline
>> function in xf86drmMode.c
>>
>> [0] I think Thierry already has a patch set that rearranges the tests
>> to use some shared headers.  Perhaps this will work well once his set
>> lands.
>>
> Fwiw I would opt for 1 or 3 (leaning towards 1), as 2 might lead to:
> - Everyone coming up with their own "helper", or
> - Ignoring DRM_MODE_PROP_EXTENDED_TYPE checking
> drmModePropertyPtr::flags, thus causing all sorts of chaos.
>
>
> Randy, Niveditha, Stuart,
>
> Does the compiler(s) used to build libdrm and friends support __inline ?

   It should.  I can't be 100% sure as there was just recently a new 
compiler release, but I would also suspect if this support was broken or 
dropped, then lots of stuff would break.


   Cheers!

 	---- Randy
Emil Velikov March 29, 2015, 4:20 p.m. UTC | #5
On 26/03/15 16:57, randyf@sibernet.com wrote:
[...]
>>> Alternatively can we:
>>>  (1) move the wrapper to xf86drmMode.h itself, or
>>>  (2) move this inline helper function out of xf86drmMode.h and into
>>> the two libdrm tests that use it (or a shared test helper .h [0])
>>>  (3) remove the inline and make drm_property_type_is a non-inline
>>> function in xf86drmMode.c
>>>
>>> [0] I think Thierry already has a patch set that rearranges the tests
>>> to use some shared headers.  Perhaps this will work well once his set
>>> lands.
>>>
>> Fwiw I would opt for 1 or 3 (leaning towards 1), as 2 might lead to:
>> - Everyone coming up with their own "helper", or
>> - Ignoring DRM_MODE_PROP_EXTENDED_TYPE checking
>> drmModePropertyPtr::flags, thus causing all sorts of chaos.
>>
>>
>> Randy, Niveditha, Stuart,
>>
>> Does the compiler(s) used to build libdrm and friends support __inline ?
> 
>   It should.  I can't be 100% sure as there was just recently a new
> compiler release, but I would also suspect if this support was broken or
> dropped, then lots of stuff would break.
> 
> 
Thanks for the information Randy.

Daniel
Based Randy's input we can go with the original solution :-) Will leave
the patch around for a week or so, for others to comment, before pushing
it. If someone else beats me to it, feel free to add:

Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>

Cheers,
Emil
Emil Velikov April 5, 2015, 2:32 p.m. UTC | #6
On 25 March 2015 at 01:01, Daniel Kurtz <djkurtz@chromium.org> wrote:
> Unfortunately, there are some users of libdrm installed headers that like
> to be built with -std=c89 -pedantic, which does not like "inline".
>
> However, __inline works.
>
Pushed to master. Thanks !

-Emil

Patch
diff mbox

diff --git a/xf86drmMode.h b/xf86drmMode.h
index 856a6bb..2d30184 100644
--- a/xf86drmMode.h
+++ b/xf86drmMode.h
@@ -240,7 +240,7 @@  typedef struct _drmModeProperty {
 	uint32_t *blob_ids; /* store the blob IDs */
 } drmModePropertyRes, *drmModePropertyPtr;
 
-static inline int drm_property_type_is(drmModePropertyPtr property,
+static __inline int drm_property_type_is(drmModePropertyPtr property,
 		uint32_t type)
 {
 	/* instanceof for props.. handles extended type vs original types: */