libcamera: base: log: Fix uninitialized variable warning
diff mbox series

Message ID 20250302020903.24077-1-laurent.pinchart@ideasonboard.com
State Accepted
Commit bb1d216113bc9080167469f08c0a8329073991d9
Headers show
Series
  • libcamera: base: log: Fix uninitialized variable warning
Related show

Commit Message

Laurent Pinchart March 2, 2025, 2:09 a.m. UTC 
gcc 13.3.0, cross-compiling from amd64 to arm64, warns about a possibly
uninitialized variable in Logger::parseLogLevel():

src/libcamera/base/log.cpp: In static member function ‘static libcamera::LogSeverity libcamera::Logger::parseLogLevel(std::string_view)’:
../../src/libcamera/base/log.cpp:694:55: error: ‘severity’ may be used uninitialized [-Werror=maybe-uninitialized]
  694 |                 if (ec != std::errc() || *end != '\0' || severity > LogFatal)
      |                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
src/libcamera/base/log.cpp:690:22: note: ‘severity’ was declared here
  690 |         unsigned int severity;
      |                      ^~~~~~~~

This appears to be a false positive, as the std::from_chars() function
should set severity value when it returns without an error. Still, the
warning is easy to solve, so fix it by initializing the severity
variable.

Fixes: 8fa119e0b50f ("libcamera: base: log: Use `std::from_chars()`")
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/base/log.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)


base-commit: c0a58b97989f7d529f1469b2c2f8705ff55d3af4

Comments

Barnabás Pőcze March 3, 2025, 10:41 a.m. UTC | #1 
Hi


2025. március 2., vasárnap 3:09 keltezéssel, Laurent Pinchart <laurent.pinchart@ideasonboard.com> írta:

> gcc 13.3.0, cross-compiling from amd64 to arm64, warns about a possibly
> uninitialized variable in Logger::parseLogLevel():

Yet it has no problems with it when not cross compiling:
https://gitlab.freedesktop.org/camera/libcamera/-/jobs/71959259 ...

> 
> src/libcamera/base/log.cpp: In static member function ‘static libcamera::LogSeverity libcamera::Logger::parseLogLevel(std::string_view)’:
> ../../src/libcamera/base/log.cpp:694:55: error: ‘severity’ may be used uninitialized [-Werror=maybe-uninitialized]
>   694 |                 if (ec != std::errc() || *end != '\0' || severity > LogFatal)
>       |                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
> src/libcamera/base/log.cpp:690:22: note: ‘severity’ was declared here
>   690 |         unsigned int severity;
>       |                      ^~~~~~~~
> 
> This appears to be a false positive, as the std::from_chars() function
> should set severity value when it returns without an error. Still, the
> warning is easy to solve, so fix it by initializing the severity
> variable.

I am pretty sure it is false positive, barring any bugs in from_chars(), unfortunate...


Regards,
Barnabás Pőcze


> 
> Fixes: 8fa119e0b50f ("libcamera: base: log: Use `std::from_chars()`")
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/libcamera/base/log.cpp | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp
> index 6a040b59290a..8bf3e1daa9c6 100644
> --- a/src/libcamera/base/log.cpp
> +++ b/src/libcamera/base/log.cpp
> @@ -687,14 +687,13 @@ LogSeverity Logger::parseLogLevel(std::string_view level)
>  		"FATAL",
>  	};
> 
> -	unsigned int severity;
> +	unsigned int severity = LogInvalid;
> 
>  	if (std::isdigit(level[0])) {
>  		auto [end, ec] = std::from_chars(level.data(), level.data() + level.size(), severity);
>  		if (ec != std::errc() || *end != '\0' || severity > LogFatal)
>  			severity = LogInvalid;
>  	} else {
> -		severity = LogInvalid;
>  		for (unsigned int i = 0; i < std::size(names); ++i) {
>  			if (names[i] == level) {
>  				severity = i;
> 
> base-commit: c0a58b97989f7d529f1469b2c2f8705ff55d3af4
> --
> Regards,
> 
> Laurent Pinchart
>
Laurent Pinchart March 3, 2025, 10:48 a.m. UTC | #2 
On Mon, Mar 03, 2025 at 11:41:58AM +0100, Barnabás Pőcze wrote:
> 2025. március 2., vasárnap 3:09 keltezéssel, Laurent Pinchart írta:
> 
> > gcc 13.3.0, cross-compiling from amd64 to arm64, warns about a possibly
> > uninitialized variable in Logger::parseLogLevel():
> 
> Yet it has no problems with it when not cross compiling:
> https://gitlab.freedesktop.org/camera/libcamera/-/jobs/71959259 ...
> 
> > 
> > src/libcamera/base/log.cpp: In static member function ‘static libcamera::LogSeverity libcamera::Logger::parseLogLevel(std::string_view)’:
> > ../../src/libcamera/base/log.cpp:694:55: error: ‘severity’ may be used uninitialized [-Werror=maybe-uninitialized]
> >   694 |                 if (ec != std::errc() || *end != '\0' || severity > LogFatal)
> >       |                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
> > src/libcamera/base/log.cpp:690:22: note: ‘severity’ was declared here
> >   690 |         unsigned int severity;
> >       |                      ^~~~~~~~
> > 
> > This appears to be a false positive, as the std::from_chars() function
> > should set severity value when it returns without an error. Still, the
> > warning is easy to solve, so fix it by initializing the severity
> > variable.
> 
> I am pretty sure it is false positive, barring any bugs in from_chars(), unfortunate...

I'm pretty sure as well, but I don't really see an alternative to this
patch for the time being.

> > Fixes: 8fa119e0b50f ("libcamera: base: log: Use `std::from_chars()`")
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  src/libcamera/base/log.cpp | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp
> > index 6a040b59290a..8bf3e1daa9c6 100644
> > --- a/src/libcamera/base/log.cpp
> > +++ b/src/libcamera/base/log.cpp
> > @@ -687,14 +687,13 @@ LogSeverity Logger::parseLogLevel(std::string_view level)
> >  		"FATAL",
> >  	};
> > 
> > -	unsigned int severity;
> > +	unsigned int severity = LogInvalid;
> > 
> >  	if (std::isdigit(level[0])) {
> >  		auto [end, ec] = std::from_chars(level.data(), level.data() + level.size(), severity);
> >  		if (ec != std::errc() || *end != '\0' || severity > LogFatal)
> >  			severity = LogInvalid;
> >  	} else {
> > -		severity = LogInvalid;
> >  		for (unsigned int i = 0; i < std::size(names); ++i) {
> >  			if (names[i] == level) {
> >  				severity = i;
> > 
> > base-commit: c0a58b97989f7d529f1469b2c2f8705ff55d3af4
Kieran Bingham March 3, 2025, 12:47 p.m. UTC | #3 
Quoting Laurent Pinchart (2025-03-03 10:48:29)
> On Mon, Mar 03, 2025 at 11:41:58AM +0100, Barnabás Pőcze wrote:
> > 2025. március 2., vasárnap 3:09 keltezéssel, Laurent Pinchart írta:
> > 
> > > gcc 13.3.0, cross-compiling from amd64 to arm64, warns about a possibly
> > > uninitialized variable in Logger::parseLogLevel():
> > 
> > Yet it has no problems with it when not cross compiling:
> > https://gitlab.freedesktop.org/camera/libcamera/-/jobs/71959259 ...
> > 
> > > 
> > > src/libcamera/base/log.cpp: In static member function ‘static libcamera::LogSeverity libcamera::Logger::parseLogLevel(std::string_view)’:
> > > ../../src/libcamera/base/log.cpp:694:55: error: ‘severity’ may be used uninitialized [-Werror=maybe-uninitialized]
> > >   694 |                 if (ec != std::errc() || *end != '\0' || severity > LogFatal)
> > >       |                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
> > > src/libcamera/base/log.cpp:690:22: note: ‘severity’ was declared here
> > >   690 |         unsigned int severity;
> > >       |                      ^~~~~~~~
> > > 
> > > This appears to be a false positive, as the std::from_chars() function
> > > should set severity value when it returns without an error. Still, the
> > > warning is easy to solve, so fix it by initializing the severity
> > > variable.
> > 
> > I am pretty sure it is false positive, barring any bugs in from_chars(), unfortunate...
> 
> I'm pretty sure as well, but I don't really see an alternative to this
> patch for the time being.
> 
> > > Fixes: 8fa119e0b50f ("libcamera: base: log: Use `std::from_chars()`")
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > >  src/libcamera/base/log.cpp | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > > 
> > > diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp
> > > index 6a040b59290a..8bf3e1daa9c6 100644
> > > --- a/src/libcamera/base/log.cpp
> > > +++ b/src/libcamera/base/log.cpp
> > > @@ -687,14 +687,13 @@ LogSeverity Logger::parseLogLevel(std::string_view level)
> > >             "FATAL",
> > >     };
> > > 
> > > -   unsigned int severity;
> > > +   unsigned int severity = LogInvalid;

False positive or not, it's not /incorrect/ to initialise the variable -
so if it's a solution it's fine with me.


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

> > > 
> > >     if (std::isdigit(level[0])) {
> > >             auto [end, ec] = std::from_chars(level.data(), level.data() + level.size(), severity);
> > >             if (ec != std::errc() || *end != '\0' || severity > LogFatal)
> > >                     severity = LogInvalid;
> > >     } else {
> > > -           severity = LogInvalid;
> > >             for (unsigned int i = 0; i < std::size(names); ++i) {
> > >                     if (names[i] == level) {
> > >                             severity = i;
> > > 
> > > base-commit: c0a58b97989f7d529f1469b2c2f8705ff55d3af4
> 
> -- 
> Regards,
> 
> Laurent Pinchart

Patch
diff mbox series 

diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp
index 6a040b59290a..8bf3e1daa9c6 100644
--- a/src/libcamera/base/log.cpp
+++ b/src/libcamera/base/log.cpp
@@ -687,14 +687,13 @@  LogSeverity Logger::parseLogLevel(std::string_view level)
 		"FATAL",
 	};
 
-	unsigned int severity;
+	unsigned int severity = LogInvalid;
 
 	if (std::isdigit(level[0])) {
 		auto [end, ec] = std::from_chars(level.data(), level.data() + level.size(), severity);
 		if (ec != std::errc() || *end != '\0' || severity > LogFatal)
 			severity = LogInvalid;
 	} else {
-		severity = LogInvalid;
 		for (unsigned int i = 0; i < std::size(names); ++i) {
 			if (names[i] == level) {
 				severity = i;