| Message ID | 20250124215806.158024-7-mzamazal@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
On Fri, Jan 24, 2025 at 10:57:57PM +0100, Milan Zamazal wrote: > SimpleCameraData::pipeConfig_ is set to the determined maxPipeConfig if > no better configuration is found. In the current code, maxPipeConfig > should be always set. But it may be easy to miss that requirement and > end up with null maxPipeConfig on contingent code changes. > > Let's add a check for nullptr to prevent segmentation fault surprises. > It doesn't harm. But that should never happen. I'm fine hardening the code, but not with an error check like this. If you think the code is too bug-prone, it should be refactored to make it less bug-prone. > Signed-off-by: Milan Zamazal <mzamazal@redhat.com> > --- > src/libcamera/pipeline/simple/simple.cpp | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index e6bbff5d..300ebbc0 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -1078,8 +1078,13 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() > } > > /* If no configuration was large enough, select the largest one. */ > - if (!pipeConfig_) > + if (!pipeConfig_) { > + if (!maxPipeConfig) { > + LOG(SimplePipeline, Error) << "No valid configuration found"; > + return Invalid; > + } > pipeConfig_ = maxPipeConfig; > + } > > LOG(SimplePipeline, Debug) > << "Picked "
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index e6bbff5d..300ebbc0 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -1078,8 +1078,13 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() } /* If no configuration was large enough, select the largest one. */ - if (!pipeConfig_) + if (!pipeConfig_) { + if (!maxPipeConfig) { + LOG(SimplePipeline, Error) << "No valid configuration found"; + return Invalid; + } pipeConfig_ = maxPipeConfig; + } LOG(SimplePipeline, Debug) << "Picked "
SimpleCameraData::pipeConfig_ is set to the determined maxPipeConfig if no better configuration is found. In the current code, maxPipeConfig should be always set. But it may be easy to miss that requirement and end up with null maxPipeConfig on contingent code changes. Let's add a check for nullptr to prevent segmentation fault surprises. It doesn't harm. Signed-off-by: Milan Zamazal <mzamazal@redhat.com> --- src/libcamera/pipeline/simple/simple.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)