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

[Laurent Pinchart]

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