[libcamera-devel] libcamera: camera: Fix unused variable compiler warning

19330 diff mbox series

When compiling with gcc 8.4.0, the compiler throws an unused variable
warning:

../src/libcamera/camera.cpp: In member function ‘libcamera::CameraConfiguration::Status libcamera::CameraConfiguration::validateColorSpaces(libcamera::CameraConfiguration::ColorSpaceFlags)’:
../src/libcamera/camera.cpp:497:19: error: unused variable ‘i’ [-Werror=unused-variable]
  for (auto [i, cfg] : utils::enumerate(config_)) {
                   ^

While the code compiles fine with 8.3.0 and 8.5.0, gcc is right here,
the 'i' variable is unused. It turns out that the code can be
simplified, as the commit that removed usage of the variable kept the
now unneeded utils::enumerate() call.

Simplify the code and fix the warning in one go.

Fixes: 13986d6ce3ab ("libcamera: camera: Fix validateColorSpaces to choose "main" colour space")
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/camera.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


base-commit: 41d6e6e5c166c267e7a15a7b0c1d930bddcbc6b8

Quoting Laurent Pinchart via libcamera-devel (2023-12-18 14:13:13)
> When compiling with gcc 8.4.0, the compiler throws an unused variable
> warning:
> 
> ../src/libcamera/camera.cpp: In member function ‘libcamera::CameraConfiguration::Status libcamera::CameraConfiguration::validateColorSpaces(libcamera::CameraConfiguration::ColorSpaceFlags)’:
> ../src/libcamera/camera.cpp:497:19: error: unused variable ‘i’ [-Werror=unused-variable]
>   for (auto [i, cfg] : utils::enumerate(config_)) {
>                    ^
> 
> While the code compiles fine with 8.3.0 and 8.5.0, gcc is right here,


Curious to only hit on one intermediate version.

> the 'i' variable is unused. It turns out that the code can be
> simplified, as the commit that removed usage of the variable kept the
> now unneeded utils::enumerate() call.
> 
> Simplify the code and fix the warning in one go.

Indeed.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> 
> Fixes: 13986d6ce3ab ("libcamera: camera: Fix validateColorSpaces to choose "main" colour space")
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/libcamera/camera.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 0ad1a4b50447..a71dc933b911 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -494,7 +494,7 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF
>         std::optional<ColorSpace> colorSpace;
>         Size size;
>  
> -       for (auto [i, cfg] : utils::enumerate(config_)) {
> +       for (StreamConfiguration &cfg : config_) {
>                 if (!cfg.colorSpace)
>                         continue;
>  
> 
> base-commit: 41d6e6e5c166c267e7a15a7b0c1d930bddcbc6b8
> -- 
> Regards,
> 
> Laurent Pinchart
>

On Wed, Dec 20, 2023 at 12:42:12PM +0000, Kieran Bingham wrote:
> Quoting Laurent Pinchart via libcamera-devel (2023-12-18 14:13:13)
> > When compiling with gcc 8.4.0, the compiler throws an unused variable
> > warning:
> > 
> > ../src/libcamera/camera.cpp: In member function ‘libcamera::CameraConfiguration::Status libcamera::CameraConfiguration::validateColorSpaces(libcamera::CameraConfiguration::ColorSpaceFlags)’:
> > ../src/libcamera/camera.cpp:497:19: error: unused variable ‘i’ [-Werror=unused-variable]
> >   for (auto [i, cfg] : utils::enumerate(config_)) {
> >                    ^
> > 
> > While the code compiles fine with 8.3.0 and 8.5.0, gcc is right here,
> 
> Curious to only hit on one intermediate version.

I think the issue was actually caused by a misconfiguration of the build
environment of the reporter of the issue. They used gcc 8.4.0 but g++
7.x. I don't want to explain this mess in the commit message though, and
as I can't easily test if g++ 8.4.0 exhibits the issue, I've left the
message as is.

> > the 'i' variable is unused. It turns out that the code can be
> > simplified, as the commit that removed usage of the variable kept the
> > now unneeded utils::enumerate() call.
> > 
> > Simplify the code and fix the warning in one go.
> 
> Indeed.
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> > Fixes: 13986d6ce3ab ("libcamera: camera: Fix validateColorSpaces to choose "main" colour space")
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  src/libcamera/camera.cpp | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> > index 0ad1a4b50447..a71dc933b911 100644
> > --- a/src/libcamera/camera.cpp
> > +++ b/src/libcamera/camera.cpp
> > @@ -494,7 +494,7 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF
> >         std::optional<ColorSpace> colorSpace;
> >         Size size;
> >  
> > -       for (auto [i, cfg] : utils::enumerate(config_)) {
> > +       for (StreamConfiguration &cfg : config_) {
> >                 if (!cfg.colorSpace)
> >                         continue;
> >  
> > 
> > base-commit: 41d6e6e5c166c267e7a15a7b0c1d930bddcbc6b8