[libcamera-devel] [PATCH] qcam: viewfinder_gl: selectColorSpace: fix maybe uninitialized warning
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sat Sep 3 02:01:34 CEST 2022
On Sat, Sep 03, 2022 at 02:32:40AM +0300, Laurent Pinchart via libcamera-devel wrote:
> On Sat, Sep 03, 2022 at 02:31:39AM +0300, Laurent Pinchart wrote:
> > Hi Marco,
> >
> > Going back to the patch now.
> >
> > On Fri, Sep 02, 2022 at 04:11:36PM +0200, Marco Felsch via libcamera-devel wrote:
> > > Fix gcc11 warning introduced in commit 251f0534 ("qcam: viewfinder_gl:
> > > Take color space into account for YUV rendering"). Fix it by making the
> > > full range as default.
> >
> > I'd like to mention the result of the investigation I detailed in a
> > separate reply in this mail thread. Would you be fine with the following
> > commit message ?
> >
> > Commit 251f0534 ("qcam: viewfinder_gl: Take color space into account for
> > YUV rendering") introduced a maybe-uninitialized warning with gcc 11 and
> > 12. The compiler is correct, as C++17 allows initializing a scoped enum
> > from an integer using direct-list-initialization, even if the integer
> > value doesn't match any of the enumerators for the scoped enum ([1]).
> >
> > Fix the warning by making the full range the default.
> >
> > https://en.cppreference.com/w/cpp/language/enum#enum_relaxed_init_cpp17
> >
> > > Fixes: 251f0534 ("qcam: viewfinder_gl: Take color space into account for YUV rendering")
>
> I also forgot to mention, Fixes tags should use 12 digits for the commit
> ID. I can also address this locally.
Also, gcc 12.2.0 still warns about the second switch statement in the
same function. I'll submit a v2 of your patch to fix that as well.
> > > Signed-off-by: Marco Felsch <m.felsch at pengutronix.de>
> > > ---
> > > src/qcam/viewfinder_gl.cpp | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp
> > > index e2aa2470..501c5bae 100644
> > > --- a/src/qcam/viewfinder_gl.cpp
> > > +++ b/src/qcam/viewfinder_gl.cpp
> > > @@ -367,6 +367,7 @@ void ViewFinderGL::selectColorSpace(const libcamera::ColorSpace &colorSpace)
> > > double offset;
> > >
> > > switch (colorSpace.range) {
> > > + default: /* Avoid gcc11 maybe-uninitialized warning */
> >
> > Given that this is a valid behaviour for the compiler, and not a false
> > positive as I initially thought, I think we can drop the comment. If you
> > don't mind I'll also move the default one line down, just right after
> > the Full case.
> >
> > There's no need to submit a v2 if you're fine with these changes, I'll
> > apply them locally.
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> >
> > > case libcamera::ColorSpace::Range::Full:
> > > offset = 0.0;
> > > break;
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list