| Message ID | 20210804102156.4636-1-kieran.bingham@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | 2c18ebb859f54a5f6e620aebc6f3abf3648b7462 |
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran, On Wed, Aug 04, 2021 at 11:21:56AM +0100, Kieran Bingham wrote: > The ThreadProxy IPA template does not implement a constructor and the > default compiler generated constructor does not initialise the private > ipa_ pointer. > > Whilst this should not be expected to be used while uninitialised, it > does get caught by static analysis for every IPA module constructed, so > lets be clean and fix it. > > Reported-by: Coverity CID=350116 > Reported-by: Coverity CID=350123 > Reported-by: Coverity CID=350140 > Reported-by: Coverity CID=350147 Ooh that's a lot of coverity fixes \o/ > Fixes: 7832e19a599e ("utils: ipc: add templates for code generation for IPC mechanism") > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > --- > .../generators/libcamera_templates/module_ipa_proxy.h.tmpl | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl > index ae168548492c..c222f5f204df 100644 > --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl > +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl > @@ -72,6 +72,11 @@ private: > class ThreadProxy : public Object > { > public: > + ThreadProxy() > + : ipa_(nullptr) > + { > + } > + > void setIPA({{interface_name}} *ipa) > { > ipa_ = ipa; > -- > 2.30.2 >
On 04/08/2021 12:26, paul.elder@ideasonboard.com wrote: > Hi Kieran, > > On Wed, Aug 04, 2021 at 11:21:56AM +0100, Kieran Bingham wrote: >> The ThreadProxy IPA template does not implement a constructor and the >> default compiler generated constructor does not initialise the private >> ipa_ pointer. >> >> Whilst this should not be expected to be used while uninitialised, it >> does get caught by static analysis for every IPA module constructed, so >> lets be clean and fix it. >> >> Reported-by: Coverity CID=350116 >> Reported-by: Coverity CID=350123 >> Reported-by: Coverity CID=350140 >> Reported-by: Coverity CID=350147 > > Ooh that's a lot of coverity fixes \o/ One for each proxy that gets constructed ;-) I made this patch some time ago and saw it in my branch and must have forgotten to send it out - so here it is ;-) > >> Fixes: 7832e19a599e ("utils: ipc: add templates for code generation for IPC mechanism") >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> Thanks. > >> --- >> .../generators/libcamera_templates/module_ipa_proxy.h.tmpl | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl >> index ae168548492c..c222f5f204df 100644 >> --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl >> +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl >> @@ -72,6 +72,11 @@ private: >> class ThreadProxy : public Object >> { >> public: >> + ThreadProxy() >> + : ipa_(nullptr) >> + { >> + } >> + >> void setIPA({{interface_name}} *ipa) >> { >> ipa_ = ipa; >> -- >> 2.30.2 >>
Hi Kieran, Thanks for the patch On 8/4/21 3:51 PM, Kieran Bingham wrote: > The ThreadProxy IPA template does not implement a constructor and the > default compiler generated constructor does not initialise the private > ipa_ pointer. > > Whilst this should not be expected to be used while uninitialised, it > does get caught by static analysis for every IPA module constructed, so > lets be clean and fix it. > > Reported-by: Coverity CID=350116 > Reported-by: Coverity CID=350123 > Reported-by: Coverity CID=350140 > Reported-by: Coverity CID=350147 > Fixes: 7832e19a599e ("utils: ipc: add templates for code generation for IPC mechanism") > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > --- > .../generators/libcamera_templates/module_ipa_proxy.h.tmpl | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl > index ae168548492c..c222f5f204df 100644 > --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl > +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl > @@ -72,6 +72,11 @@ private: > class ThreadProxy : public Object > { > public: > + ThreadProxy() > + : ipa_(nullptr) > + { > + } > + > void setIPA({{interface_name}} *ipa) > { > ipa_ = ipa;
Hi Kieran, Thank you for the patch. On Wed, Aug 04, 2021 at 11:21:56AM +0100, Kieran Bingham wrote: > The ThreadProxy IPA template does not implement a constructor and the > default compiler generated constructor does not initialise the private > ipa_ pointer. > > Whilst this should not be expected to be used while uninitialised, it > does get caught by static analysis for every IPA module constructed, so > lets be clean and fix it. > > Reported-by: Coverity CID=350116 > Reported-by: Coverity CID=350123 > Reported-by: Coverity CID=350140 > Reported-by: Coverity CID=350147 > Fixes: 7832e19a599e ("utils: ipc: add templates for code generation for IPC mechanism") > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > .../generators/libcamera_templates/module_ipa_proxy.h.tmpl | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl > index ae168548492c..c222f5f204df 100644 > --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl > +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl > @@ -72,6 +72,11 @@ private: > class ThreadProxy : public Object > { > public: > + ThreadProxy() > + : ipa_(nullptr) > + { > + } > + > void setIPA({{interface_name}} *ipa) > { > ipa_ = ipa; Note that this doesn't fix a bug, as we always call setIPA() before using ipa_. As it's easy to please coverity and there's no drawback (this is clearly not a hot path), it's good to keep the warnings count low. Another option would be diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl index ae168548492c..d535e0417433 100644 --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl @@ -102,7 +102,7 @@ private: {%- endfor %} private: - {{interface_name}} *ipa_; + {{interface_name}} *ipa_ = nullptr; }; Thread thread_; I wonder if the constructor would still be inline in that case. It likely doesn't matter much as it's very simple anyway. Either way, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl index ae168548492c..c222f5f204df 100644 --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl @@ -72,6 +72,11 @@ private: class ThreadProxy : public Object { public: + ThreadProxy() + : ipa_(nullptr) + { + } + void setIPA({{interface_name}} *ipa) { ipa_ = ipa;
The ThreadProxy IPA template does not implement a constructor and the default compiler generated constructor does not initialise the private ipa_ pointer. Whilst this should not be expected to be used while uninitialised, it does get caught by static analysis for every IPA module constructed, so lets be clean and fix it. Reported-by: Coverity CID=350116 Reported-by: Coverity CID=350123 Reported-by: Coverity CID=350140 Reported-by: Coverity CID=350147 Fixes: 7832e19a599e ("utils: ipc: add templates for code generation for IPC mechanism") Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- .../generators/libcamera_templates/module_ipa_proxy.h.tmpl | 5 +++++ 1 file changed, 5 insertions(+)