[libcamera-devel] libcamera: pipeline: simple: Check if converter_ is a nullptr

[Laurent Pinchart]

On Fri, Feb 24, 2023 at 11:52:35AM +0530, Umang Jain via libcamera-devel wrote:
> Hi Suhrid,
> 
> Thank you for the patch.
> 
> On 2/24/23 2:17 AM, Suhrid Subramaniam via libcamera-devel wrote:
> > - If no converter is found, converter_ becomes a nullptr and
> > !converter_->isValid() causes a segmentation fault.
> > - Avoid this by checking if converter_ is a nullptr.
> >
> > Signed-off-by: Suhrid Subramaniam <suhrid.subramaniam at mediatek.com>
> > ---
> >   src/libcamera/pipeline/simple/simple.cpp | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> > index 24ded4db..2423ec10 100644
> > --- a/src/libcamera/pipeline/simple/simple.cpp
> > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > @@ -493,7 +493,7 @@ int SimpleCameraData::init()
> >   	MediaDevice *converter = pipe->converter();
> >   	if (converter) {
> >   		converter_ = ConverterFactoryBase::create(converter);
> > -		if (!converter_->isValid()) {
> > +		if (!converter_) {
> 
> it seems this change will drop the validity check so maybe :
>          if (!converter_ && converter_->isValid()) {
> 
> The error message below might be need to get tweaked a bit.
> 
> On the other hand, I wonder if ConverterFactoryBase::create() should be 
> allowed? to return a non-valid converter pointer or not.

I don't see a use case for that, so I'd rather check for !valid in
::create() and return nullptr in that case.

> >   			LOG(SimplePipeline, Warning)
> >   				<< "Failed to create converter, disabling format conversion";
> >   			converter_.reset();

-- 
Regards,

Laurent Pinchart