| Message ID | 20191123083651.6944-1-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, On Sat, Nov 23, 2019 at 10:36:51AM +0200, Laurent Pinchart wrote: > gcc 8 and 9 complain about the OptionValue::integer_ member being > possibly used initialized when compiled in release mode. I haven't been > able to find where this could be the case, and the compiler error > message isn't helpful: > > In file included from ../../src/cam/options.cpp:14: > ../../src/cam/options.h: In member function ‘bool OptionsBase<T>::parseValue(const T&, const Option&, const char*) [with T = std::__cxx11::basic_string<char>]’: > ../../src/cam/options.h:84:7: error: ‘<anonymous>.OptionValue::integer_’ may be used uninitialized in this function [-Werror=maybe-uninitialized] > class OptionValue > ^~~~~~~~~~~ > ../../src/cam/options.h: In member function ‘bool OptionsBase<T>::parseValue(const T&, const Option&, const char*) [with T = int]’: > ../../src/cam/options.h:84:7: error: ‘<anonymous>.OptionValue::integer_’ may be used uninitialized in this function [-Werror=maybe-uninitialized] > class OptionValue > ^~~~~~~~~~~ > > Furthermore valgrind doesn't report any issue. This is likely a false > positive, but fix it nonetheless as the fix is cheap. Possibly, but let's silence it anyway. It won't hurt for sure. Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/cam/options.cpp | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/src/cam/options.cpp b/src/cam/options.cpp > index 7e2dfa636ccf..186e853c8294 100644 > --- a/src/cam/options.cpp > +++ b/src/cam/options.cpp > @@ -260,17 +260,17 @@ OptionValue::OptionValue(int value) > } > > OptionValue::OptionValue(const char *value) > - : type_(ValueString), string_(value) > + : type_(ValueString), integer_(0), string_(value) > { > } > > OptionValue::OptionValue(const std::string &value) > - : type_(ValueString), string_(value) > + : type_(ValueString), integer_(0), string_(value) > { > } > > OptionValue::OptionValue(const KeyValueParser::Options &value) > - : type_(ValueKeyValue), keyValues_(value) > + : type_(ValueKeyValue), integer_(0), keyValues_(value) > { > } > > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Laurent, On 23/11/2019 10:16, Jacopo Mondi wrote: > Hi Laurent, > > On Sat, Nov 23, 2019 at 10:36:51AM +0200, Laurent Pinchart wrote: >> gcc 8 and 9 complain about the OptionValue::integer_ member being >> possibly used initialized when compiled in release mode. I haven't been >> able to find where this could be the case, and the compiler error >> message isn't helpful: >> >> In file included from ../../src/cam/options.cpp:14: >> ../../src/cam/options.h: In member function ‘bool OptionsBase<T>::parseValue(const T&, const Option&, const char*) [with T = std::__cxx11::basic_string<char>]’: >> ../../src/cam/options.h:84:7: error: ‘<anonymous>.OptionValue::integer_’ may be used uninitialized in this function [-Werror=maybe-uninitialized] >> class OptionValue >> ^~~~~~~~~~~ >> ../../src/cam/options.h: In member function ‘bool OptionsBase<T>::parseValue(const T&, const Option&, const char*) [with T = int]’: >> ../../src/cam/options.h:84:7: error: ‘<anonymous>.OptionValue::integer_’ may be used uninitialized in this function [-Werror=maybe-uninitialized] >> class OptionValue >> ^~~~~~~~~~~ >> >> Furthermore valgrind doesn't report any issue. This is likely a false >> positive, but fix it nonetheless as the fix is cheap. > > Possibly, but let's silence it anyway. It won't hurt for sure. > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> OptionValue is indeed leaving the integer_ uninitialised in it's non-int constructors, though I can't see that it is being used without calling through the (int) constructor which /would/ initialise it. Unless perhaps it's because of one of the templated base class usages? > Template class OptionsBase<int>; > class OptionsParser > { > public: > class Options : public OptionsBase<int> > Anyway, initialising it seems the 'correct' thing to do, though I really can't see that it could be used. But it's likely hard for the compiler to 'prove' that. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > Thanks > j > >> >> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> --- >> src/cam/options.cpp | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/src/cam/options.cpp b/src/cam/options.cpp >> index 7e2dfa636ccf..186e853c8294 100644 >> --- a/src/cam/options.cpp >> +++ b/src/cam/options.cpp Should this be initialised on the default constructor too for completeness? OptionValue::OptionValue() : type_(ValueNone) { } >> @@ -260,17 +260,17 @@ OptionValue::OptionValue(int value) >> } >> >> OptionValue::OptionValue(const char *value) >> - : type_(ValueString), string_(value) >> + : type_(ValueString), integer_(0), string_(value) >> { >> } >> >> OptionValue::OptionValue(const std::string &value) >> - : type_(ValueString), string_(value) >> + : type_(ValueString), integer_(0), string_(value) >> { >> } >> >> OptionValue::OptionValue(const KeyValueParser::Options &value) >> - : type_(ValueKeyValue), keyValues_(value) >> + : type_(ValueKeyValue), integer_(0), keyValues_(value) >> { >> } >> >> -- >> Regards, >> >> Laurent Pinchart >> >> _______________________________________________ >> libcamera-devel mailing list >> libcamera-devel@lists.libcamera.org >> https://lists.libcamera.org/listinfo/libcamera-devel >> >> _______________________________________________ >> libcamera-devel mailing list >> libcamera-devel@lists.libcamera.org >> https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/src/cam/options.cpp b/src/cam/options.cpp index 7e2dfa636ccf..186e853c8294 100644 --- a/src/cam/options.cpp +++ b/src/cam/options.cpp @@ -260,17 +260,17 @@ OptionValue::OptionValue(int value) } OptionValue::OptionValue(const char *value) - : type_(ValueString), string_(value) + : type_(ValueString), integer_(0), string_(value) { } OptionValue::OptionValue(const std::string &value) - : type_(ValueString), string_(value) + : type_(ValueString), integer_(0), string_(value) { } OptionValue::OptionValue(const KeyValueParser::Options &value) - : type_(ValueKeyValue), keyValues_(value) + : type_(ValueKeyValue), integer_(0), keyValues_(value) { }
gcc 8 and 9 complain about the OptionValue::integer_ member being possibly used initialized when compiled in release mode. I haven't been able to find where this could be the case, and the compiler error message isn't helpful: In file included from ../../src/cam/options.cpp:14: ../../src/cam/options.h: In member function ‘bool OptionsBase<T>::parseValue(const T&, const Option&, const char*) [with T = std::__cxx11::basic_string<char>]’: ../../src/cam/options.h:84:7: error: ‘<anonymous>.OptionValue::integer_’ may be used uninitialized in this function [-Werror=maybe-uninitialized] class OptionValue ^~~~~~~~~~~ ../../src/cam/options.h: In member function ‘bool OptionsBase<T>::parseValue(const T&, const Option&, const char*) [with T = int]’: ../../src/cam/options.h:84:7: error: ‘<anonymous>.OptionValue::integer_’ may be used uninitialized in this function [-Werror=maybe-uninitialized] class OptionValue ^~~~~~~~~~~ Furthermore valgrind doesn't report any issue. This is likely a false positive, but fix it nonetheless as the fix is cheap. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/cam/options.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)