[libcamera-devel] [PATCH] qcam: viewfinder_gl: selectColorSpace: fix maybe uninitialized warning

[Laurent Pinchart]

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.

> > 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