| Message ID | 1427245262-28865-1-git-send-email-djkurtz@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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 >
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 :)
> >> 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
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
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
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: */
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(-)