| Message ID | 20200515124245.18040-5-email@uajain.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Umang, On 15/05/2020 13:42, Umang Jain wrote: > Fail and return the Camera::configure() operation if any > of the stream turns out to be a nullptr even after the > PipelineHandler handler seems to have configured the config > successfully. This prevents a null-dereference below in the > loop. > > Pointed out by Coverity DefectId=279069 We've set a style for reporting tags as: Reported-by: Coverity CID=279069 I've updated the tags in this branch when applying to my local tree, so no need for you to repost, just an FYI. We should probably create a checkstyle rule for it... Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > Signed-off-by: Umang Jain <email@uajain.com> > --- > src/libcamera/camera.cpp | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > index 8c3bb2c..9d2607b 100644 > --- a/src/libcamera/camera.cpp > +++ b/src/libcamera/camera.cpp > @@ -777,9 +777,12 @@ int Camera::configure(CameraConfiguration *config) > p_->activeStreams_.clear(); > for (const StreamConfiguration &cfg : *config) { > Stream *stream = cfg.stream(); > - if (!stream) > + if (!stream) { > LOG(Camera, Fatal) > << "Pipeline handler failed to update stream configuration"; > + p_->activeStreams_.clear(); > + return -EINVAL; > + } > > stream->configuration_ = cfg; > p_->activeStreams_.insert(stream); >
Hi Umang, Thank you for the patch. On Fri, May 15, 2020 at 12:42:54PM +0000, Umang Jain wrote: > Fail and return the Camera::configure() operation if any > of the stream turns out to be a nullptr even after the > PipelineHandler handler seems to have configured the config > successfully. This prevents a null-dereference below in the > loop. > > Pointed out by Coverity DefectId=279069 > > Signed-off-by: Umang Jain <email@uajain.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/libcamera/camera.cpp | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > index 8c3bb2c..9d2607b 100644 > --- a/src/libcamera/camera.cpp > +++ b/src/libcamera/camera.cpp > @@ -777,9 +777,12 @@ int Camera::configure(CameraConfiguration *config) > p_->activeStreams_.clear(); > for (const StreamConfiguration &cfg : *config) { > Stream *stream = cfg.stream(); > - if (!stream) > + if (!stream) { > LOG(Camera, Fatal) > << "Pipeline handler failed to update stream configuration"; > + p_->activeStreams_.clear(); > + return -EINVAL; > + } > > stream->configuration_ = cfg; > p_->activeStreams_.insert(stream);
diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 8c3bb2c..9d2607b 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -777,9 +777,12 @@ int Camera::configure(CameraConfiguration *config) p_->activeStreams_.clear(); for (const StreamConfiguration &cfg : *config) { Stream *stream = cfg.stream(); - if (!stream) + if (!stream) { LOG(Camera, Fatal) << "Pipeline handler failed to update stream configuration"; + p_->activeStreams_.clear(); + return -EINVAL; + } stream->configuration_ = cfg; p_->activeStreams_.insert(stream);
Fail and return the Camera::configure() operation if any of the stream turns out to be a nullptr even after the PipelineHandler handler seems to have configured the config successfully. This prevents a null-dereference below in the loop. Pointed out by Coverity DefectId=279069 Signed-off-by: Umang Jain <email@uajain.com> --- src/libcamera/camera.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)