[libcamera-devel] [libcamera-internal] [PATCH] android/mm/cros_camera_buffer: Mark cameraBuffer unused parameter

[Laurent Pinchart]

Hello,

On Thu, Aug 05, 2021 at 05:31:31PM +0900, Hirokazu Honda wrote:
> On Thu, Aug 5, 2021 at 5:16 PM Umang Jain <umang.jain at ideasonboard.com> wrote:
> >
> > Hi Hiro,
> >
> > Apologies for the unnecessary churn, it seems we are not catching
> > -Wunused-parameter somehow
> >
> > I had to:
> >
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -43,6 +43,7 @@ endif
> >
> >   common_arguments = [
> >       '-Wshadow',
> > +    '-Wunused-parameter',
> >       '-include', 'config.h',
> >   ]
> >
> > to reproduce. So:
> 
> Oh, I thought we had it as we have been using [[maybe_unused]].
> Thanks for finding it. Would you mind uploading patch series of adding
> the parameter?

We set the default warning level in meson.build to 2:

project('libcamera', 'c', 'cpp',
    meson_version : '>= 0.55',
    version : '0.0.0',
    default_options : [
        'werror=true',
        'warning_level=2',
        'cpp_std=c++17',
    ],
    license : 'LGPL 2.1+')

This maps to -Wall -Wextra, which enables -Wunused-parameter. We thus
don't need to add -Wunused-parameter explicitly.

This build breakage happened because I haven't compiled the series on
Chrome OS, so src/android/mm/cros_camera_buffer.cpp hasn't been
compile-tested :-S

> > Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
> >
> > On 8/5/21 1:31 PM, Hirokazu Honda wrote:
> > > On Thu, Aug 5, 2021 at 4:59 PM Umang Jain wrote:
> > >> On 8/5/21 12:31 PM, Hirokazu Honda wrote:
> > >>> On Wed, Aug 4, 2021 at 1:48 PM Hirokazu Honda wrote:
> > >>>> Hi Umang,
> > >>>>
> > >>>> Yes, I am building inside CrOS chroot. It makes me compiling
> > >>>> cros_camera_buffer.cpp, not generic_camera_buffer.cpp.
> > >>
> > >> yes, cros_camera_buffer.cpp is what's supposed to get compiled given
> > >> what we have in the ebuild.
> > >>
> > >>>> The compilation failure is caused by Wunused-parameter.
> > >>>> I expect the warning flag is also used in a standalone build too.
> > >>>> I attached the build log.
> > >>>>
> > >>>> -Hiro
> > >>>>
> > >>>> On Wed, Aug 4, 2021 at 1:25 PM Umang Jain wrote:
> > >>>>> On 8/4/21 9:09 AM, Hirokazu Honda wrote:
> > >>>>>> This fixes the compilation error introduced by the commit
> > >>>>>> 33dd4fab9d39726be4fcbd300a27f2640be1cd6f.
> > >>>>>
> > >>>>> Would you mind sharing your build log? Are you building inside CrOS
> > >>>>> chroot or standalone?
> > >>>>>
> > >>>>> I haven't seen any compilation failures recently, especially on master
> > >>>>> commits.
> > >>>>>
> > >>>>> Thanks!
> > >>>>>
> > >>>>>> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> > >>>>>> ---
> > >>>>>>     src/android/mm/cros_camera_buffer.cpp | 2 +-
> > >>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)
> > >>>>>>
> > >>>>>> diff --git a/src/android/mm/cros_camera_buffer.cpp b/src/android/mm/cros_camera_buffer.cpp
> > >>>>>> index 437502fb..e8783ff8 100644
> > >>>>>> --- a/src/android/mm/cros_camera_buffer.cpp
> > >>>>>> +++ b/src/android/mm/cros_camera_buffer.cpp
> > >>>>>> @@ -44,7 +44,7 @@ private:
> > >>>>>>         } mem;
> > >>>>>>     };
> > >>>>>>
> > >>>>>> -CameraBuffer::Private::Private(CameraBuffer *cameraBuffer,
> > >>>>>> +CameraBuffer::Private::Private([[maybe_unused]] CameraBuffer *cameraBuffer,
> > >>
> > >> I suspect you have a dirty commit lingering around? What I see on master
> > >> is that the cameraBuffer is indeed used, which is why [[maybe_unused]]
> > >> is absent in the first place.
> > >>
> > >> https://chromium.googlesource.com/chromiumos/third_party/libcamera/+/refs/heads/master/src/android/mm/cros_camera_buffer.cpp#55
> > >
> > > It is camera3Buffer, not cameraBuffer. ;)
> > >
> > >>>>>>                                buffer_handle_t camera3Buffer,
> > >>>>>>                                [[maybe_unused]] int flags)
> > >>>>>>         : handle_(camera3Buffer), numPlanes_(0), valid_(false),

-- 
Regards,

Laurent Pinchart