| Message ID | 20200430003604.2423018-2-niklas.soderlund@ragnatech.se |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Niklas, Thank you for the patch. On Thu, Apr 30, 2020 at 02:36:00AM +0200, Niklas Söderlund wrote: > If the camera can not generate a configuration from the requested roles > it returns a nullptr which leads to a nullptr dereference. Fix this by > adding a check that the camera generated a configuration before trying > to access it. > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/qcam/main_window.cpp | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp > index ee779728fc630da8..b683c2e00d317307 100644 > --- a/src/qcam/main_window.cpp > +++ b/src/qcam/main_window.cpp > @@ -293,6 +293,10 @@ int MainWindow::startCapture() > > /* Configure the camera. */ > config_ = camera_->generateConfiguration(roles); > + if (!config_) { > + qWarning() << "Failed to generate configuration from roles"; > + return -EINVAL; > + } This is certainly better than a crash, so Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> but what happens in the GUI ? > > StreamConfiguration &cfg = config_->at(0); >
Hi Laurent, Thanks for your feedback. On 2020-04-30 03:44:14 +0300, Laurent Pinchart wrote: > Hi Niklas, > > Thank you for the patch. > > On Thu, Apr 30, 2020 at 02:36:00AM +0200, Niklas Söderlund wrote: > > If the camera can not generate a configuration from the requested roles > > it returns a nullptr which leads to a nullptr dereference. Fix this by > > adding a check that the camera generated a configuration before trying > > to access it. > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > --- > > src/qcam/main_window.cpp | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp > > index ee779728fc630da8..b683c2e00d317307 100644 > > --- a/src/qcam/main_window.cpp > > +++ b/src/qcam/main_window.cpp > > @@ -293,6 +293,10 @@ int MainWindow::startCapture() > > > > /* Configure the camera. */ > > config_ = camera_->generateConfiguration(roles); > > + if (!config_) { > > + qWarning() << "Failed to generate configuration from roles"; > > + return -EINVAL; > > + } > > This is certainly better than a crash, so > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > but what happens in the GUI ? It refuses to start to stream but does not lock-up, same as for the error cases bellow this one where -EINVAL is already returned. > > > > > StreamConfiguration &cfg = config_->at(0); > > > > -- > Regards, > > Laurent Pinchart
Hi Niklas, On Fri, May 01, 2020 at 04:05:59AM +0200, Niklas Söderlund wrote: > On 2020-04-30 03:44:14 +0300, Laurent Pinchart wrote: > > On Thu, Apr 30, 2020 at 02:36:00AM +0200, Niklas Söderlund wrote: > > > If the camera can not generate a configuration from the requested roles > > > it returns a nullptr which leads to a nullptr dereference. Fix this by > > > adding a check that the camera generated a configuration before trying > > > to access it. > > > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > > --- > > > src/qcam/main_window.cpp | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp > > > index ee779728fc630da8..b683c2e00d317307 100644 > > > --- a/src/qcam/main_window.cpp > > > +++ b/src/qcam/main_window.cpp > > > @@ -293,6 +293,10 @@ int MainWindow::startCapture() > > > > > > /* Configure the camera. */ > > > config_ = camera_->generateConfiguration(roles); > > > + if (!config_) { > > > + qWarning() << "Failed to generate configuration from roles"; > > > + return -EINVAL; > > > + } > > > > This is certainly better than a crash, so > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > but what happens in the GUI ? > > It refuses to start to stream but does not lock-up, same as for the > error cases bellow this one where -EINVAL is already returned. We probably want to have a way to display messages in the GUI. Not something for this patch series of course. I was thinking of a zone at the bottom of the window with some sort of log (or even a single message in a status bar). > > > > > > StreamConfiguration &cfg = config_->at(0); > > >
diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index ee779728fc630da8..b683c2e00d317307 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -293,6 +293,10 @@ int MainWindow::startCapture() /* Configure the camera. */ config_ = camera_->generateConfiguration(roles); + if (!config_) { + qWarning() << "Failed to generate configuration from roles"; + return -EINVAL; + } StreamConfiguration &cfg = config_->at(0);
If the camera can not generate a configuration from the requested roles it returns a nullptr which leads to a nullptr dereference. Fix this by adding a check that the camera generated a configuration before trying to access it. Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> --- src/qcam/main_window.cpp | 4 ++++ 1 file changed, 4 insertions(+)