| Message ID | 1521236882-21120-1-git-send-email-john.stultz@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hey John, On Fri, Mar 16, 2018 at 10:48 PM, John Stultz <john.stultz@linaro.org> wrote: > In trying to integrate the new gralloc_handle.h with the > drm_hwcomposer, I started seeing the following compilation > errors: > > In file included from external/drm_hwcomposer/platformdrmgeneric.cpp:28: > external/libdrm/android/gralloc_handle.h:108:9: error: cannot initialize return object of type 'native_handle_t *' (aka 'native_handle *') with an lvalue of type 'struct gralloc_handle_t *' > return handle; > ^~~~~~ > 1 error generated. > > This seems to be due to the gralloc_handle_create() definition > claiming to return a native_handle_t * type, rather then a > gralloc_handle_t *, which is what the code actually returns. > > This function isn't actually used in the current drm_hwcomposer, > so I'm not totally sure that this fix is correct (rather then > returning the gralloc_handle_t's embadedded native_handle_t ptr). This changed in 009634e49309 ("android: fix gralloc_handle_create() problems") to make the return value from gralloc_handle_create opaque to its user. Since the only one that would use gralloc_handle_create is the gralloc implementation itself (and libdrm isn't one) and using getters/setters was rejected (IIRC), I would think there is no reason to obscure the return type. Since buffer_handle_t is native_handle_t* is gralloc_handle_t* it's also correct. The alternative to changing the declaration here is returning nhandle instead of handle. Adding Rob Herring who authored 009634e49309. Thanks, Stefan
Hey John, On 03/16/2018 10:48 PM, John Stultz wrote: > In trying to integrate the new gralloc_handle.h with the > drm_hwcomposer, I started seeing the following compilation > errors: > > In file included from external/drm_hwcomposer/platformdrmgeneric.cpp:28: > external/libdrm/android/gralloc_handle.h:108:9: error: cannot initialize return object of type 'native_handle_t *' (aka 'native_handle *') with an lvalue of type 'struct gralloc_handle_t *' > return handle; > ^~~~~~ > 1 error generated. > > This seems to be due to the gralloc_handle_create() definition > claiming to return a native_handle_t * type, rather then a > gralloc_handle_t *, which is what the code actually returns. > > This function isn't actually used in the current drm_hwcomposer, > so I'm not totally sure that this fix is correct (rather then > returning the gralloc_handle_t's embadedded native_handle_t ptr). > > But it seems something like this is needed. > > Cc: Robert Foss <robert.foss@collabora.com> > Cc: Rob Herring <robh@kernel.org> > Cc: Sean Paul <seanpaul@google.com> > Signed-off-by: John Stultz <john.stultz@linaro.org> > --- > android/gralloc_handle.h | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/android/gralloc_handle.h b/android/gralloc_handle.h > index 9cb5a5d..6e925c9 100644 > --- a/android/gralloc_handle.h > +++ b/android/gralloc_handle.h > @@ -84,7 +84,7 @@ static inline struct gralloc_handle_t *gralloc_handle(buffer_handle_t handle) > /** > * Create a buffer handle. > */ > -static inline native_handle_t *gralloc_handle_create(int32_t width, > +static inline gralloc_handle_t *gralloc_handle_create(int32_t width, > int32_t height, > int32_t hal_format, > int32_t usage) > @@ -107,5 +107,4 @@ static inline native_handle_t *gralloc_handle_create(int32_t width, > > return handle; > } > - > #endif > The gralloc_handle_t regturn type seems to be the correct one to me. This patch is: Reviewed-by: Robert Foss <robert.foss@collabora.com>
On Sat, Mar 17, 2018 at 2:33 PM, Stefan Schake <stschake@gmail.com> wrote: > Hey John, > > On Fri, Mar 16, 2018 at 10:48 PM, John Stultz <john.stultz@linaro.org> wrote: >> In trying to integrate the new gralloc_handle.h with the >> drm_hwcomposer, I started seeing the following compilation >> errors: >> >> In file included from external/drm_hwcomposer/platformdrmgeneric.cpp:28: >> external/libdrm/android/gralloc_handle.h:108:9: error: cannot initialize return object of type 'native_handle_t *' (aka 'native_handle *') with an lvalue of type 'struct gralloc_handle_t *' >> return handle; >> ^~~~~~ >> 1 error generated. Somehow this is not throwing an error for me... >> This seems to be due to the gralloc_handle_create() definition >> claiming to return a native_handle_t * type, rather then a >> gralloc_handle_t *, which is what the code actually returns. >> >> This function isn't actually used in the current drm_hwcomposer, >> so I'm not totally sure that this fix is correct (rather then >> returning the gralloc_handle_t's embadedded native_handle_t ptr). > > This changed in 009634e49309 ("android: fix gralloc_handle_create() problems") > to make the return value from gralloc_handle_create opaque to its user. > > Since the only one that would use gralloc_handle_create is the gralloc > implementation itself (and libdrm isn't one) and using getters/setters was > rejected (IIRC), I would think there is no reason to obscure the return type. Not rejected, just not required at the start and not in a lib. And I am working on adding helper functions and I'm using buffer_handle_t everywhere. > Since buffer_handle_t is native_handle_t* is gralloc_handle_t* it's also > correct. The alternative to changing the declaration here is returning > nhandle instead of handle. BTW, returning buffer_handle_t doesn't work though because it is const. Rob
On Wed, Mar 28, 2018 at 10:19 AM, Rob Herring <robh@kernel.org> wrote: > On Sat, Mar 17, 2018 at 2:33 PM, Stefan Schake <stschake@gmail.com> wrote: >> Hey John, >> >> On Fri, Mar 16, 2018 at 10:48 PM, John Stultz <john.stultz@linaro.org> wrote: >>> In trying to integrate the new gralloc_handle.h with the >>> drm_hwcomposer, I started seeing the following compilation >>> errors: >>> >>> In file included from external/drm_hwcomposer/platformdrmgeneric.cpp:28: >>> external/libdrm/android/gralloc_handle.h:108:9: error: cannot initialize return object of type 'native_handle_t *' (aka 'native_handle *') with an lvalue of type 'struct gralloc_handle_t *' >>> return handle; >>> ^~~~~~ >>> 1 error generated. > > Somehow this is not throwing an error for me... NM. I have it fixed in a subsequent change in my tree...
2018-03-28 23:22 GMT+08:00 Rob Herring <robh@kernel.org>: > On Wed, Mar 28, 2018 at 10:19 AM, Rob Herring <robh@kernel.org> wrote: >> On Sat, Mar 17, 2018 at 2:33 PM, Stefan Schake <stschake@gmail.com> wrote: >>> Hey John, >>> >>> On Fri, Mar 16, 2018 at 10:48 PM, John Stultz <john.stultz@linaro.org> wrote: >>>> In trying to integrate the new gralloc_handle.h with the >>>> drm_hwcomposer, I started seeing the following compilation >>>> errors: >>>> >>>> In file included from external/drm_hwcomposer/platformdrmgeneric.cpp:28: >>>> external/libdrm/android/gralloc_handle.h:108:9: error: cannot initialize return object of type 'native_handle_t *' (aka 'native_handle *') with an lvalue of type 'struct gralloc_handle_t *' >>>> return handle; >>>> ^~~~~~ >>>> 1 error generated. >> >> Somehow this is not throwing an error for me... > > NM. I have it fixed in a subsequent change in my tree... Hi, I saw the same error after updated to libdrm master branch. Could John's fix be merged? Or maybe Rob has a better fix?
Hey Chih-Wei, Thanks for the poke, the patch has been pushed now. Rob. On 04/03/2018 04:58 AM, Chih-Wei Huang wrote: > 2018-03-28 23:22 GMT+08:00 Rob Herring <robh@kernel.org>: >> On Wed, Mar 28, 2018 at 10:19 AM, Rob Herring <robh@kernel.org> wrote: >>> On Sat, Mar 17, 2018 at 2:33 PM, Stefan Schake <stschake@gmail.com> wrote: >>>> Hey John, >>>> >>>> On Fri, Mar 16, 2018 at 10:48 PM, John Stultz <john.stultz@linaro.org> wrote: >>>>> In trying to integrate the new gralloc_handle.h with the >>>>> drm_hwcomposer, I started seeing the following compilation >>>>> errors: >>>>> >>>>> In file included from external/drm_hwcomposer/platformdrmgeneric.cpp:28: >>>>> external/libdrm/android/gralloc_handle.h:108:9: error: cannot initialize return object of type 'native_handle_t *' (aka 'native_handle *') with an lvalue of type 'struct gralloc_handle_t *' >>>>> return handle; >>>>> ^~~~~~ >>>>> 1 error generated. >>> >>> Somehow this is not throwing an error for me... >> >> NM. I have it fixed in a subsequent change in my tree... > > Hi, > I saw the same error after updated to libdrm master branch. > Could John's fix be merged? > Or maybe Rob has a better fix? > >
diff --git a/android/gralloc_handle.h b/android/gralloc_handle.h index 9cb5a5d..6e925c9 100644 --- a/android/gralloc_handle.h +++ b/android/gralloc_handle.h @@ -84,7 +84,7 @@ static inline struct gralloc_handle_t *gralloc_handle(buffer_handle_t handle) /** * Create a buffer handle. */ -static inline native_handle_t *gralloc_handle_create(int32_t width, +static inline gralloc_handle_t *gralloc_handle_create(int32_t width, int32_t height, int32_t hal_format, int32_t usage) @@ -107,5 +107,4 @@ static inline native_handle_t *gralloc_handle_create(int32_t width, return handle; } - #endif
In trying to integrate the new gralloc_handle.h with the drm_hwcomposer, I started seeing the following compilation errors: In file included from external/drm_hwcomposer/platformdrmgeneric.cpp:28: external/libdrm/android/gralloc_handle.h:108:9: error: cannot initialize return object of type 'native_handle_t *' (aka 'native_handle *') with an lvalue of type 'struct gralloc_handle_t *' return handle; ^~~~~~ 1 error generated. This seems to be due to the gralloc_handle_create() definition claiming to return a native_handle_t * type, rather then a gralloc_handle_t *, which is what the code actually returns. This function isn't actually used in the current drm_hwcomposer, so I'm not totally sure that this fix is correct (rather then returning the gralloc_handle_t's embadedded native_handle_t ptr). But it seems something like this is needed. Cc: Robert Foss <robert.foss@collabora.com> Cc: Rob Herring <robh@kernel.org> Cc: Sean Paul <seanpaul@google.com> Signed-off-by: John Stultz <john.stultz@linaro.org> --- android/gralloc_handle.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)