[v1] pipeline: rpi: Fix potential empty optional read

23012 diff mbox series

If `!target`, then `*target` is undefined behaviour, so check if the optional
is empty when printing the error message. Simplify the check as well.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 src/libcamera/pipeline/rpi/vc4/vc4.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--
2.49.0

Hi Barnabás,

Thank you for the patch.

On Tue, Mar 25, 2025 at 03:07:45PM +0100, Barnabás Pőcze wrote:
> If `!target`, then `*target` is undefined behaviour, so check if the optional
> is empty when printing the error message. Simplify the check as well.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  src/libcamera/pipeline/rpi/vc4/vc4.cpp | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> index fd8d84b14..fe910bdf2 100644
> --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> @@ -510,9 +510,9 @@ int Vc4CameraData::platformPipelineConfigure(const std::unique_ptr<YamlObject> &
>  	}
> 
>  	std::optional<std::string> target = (*root)["target"].get<std::string>();
> -	if (!target || *target != "bcm2835") {
> +	if (target != "bcm2835") {
>  		LOG(RPI, Error) << "Unexpected target reported: expected \"bcm2835\", got "
> -				<< *target;
> +				<< (target ? target->c_str() : "(unknown)");
>  		return -EINVAL;
>  	}
>

Hi Naush,

A relatively small patch for RPi here, LGTM, but are you happy with
this as a fix?

Quoting Laurent Pinchart (2025-03-26 02:00:19)
> Hi Barnabás,
> 
> Thank you for the patch.
> 
> On Tue, Mar 25, 2025 at 03:07:45PM +0100, Barnabás Pőcze wrote:
> > If `!target`, then `*target` is undefined behaviour, so check if the optional
> > is empty when printing the error message. Simplify the check as well.
> > 

Any fixes tag suitable? (would make this list in the fixes log of the
libcamera release)

--
Kieran


> > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> > ---
> >  src/libcamera/pipeline/rpi/vc4/vc4.cpp | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> > index fd8d84b14..fe910bdf2 100644
> > --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> > +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> > @@ -510,9 +510,9 @@ int Vc4CameraData::platformPipelineConfigure(const std::unique_ptr<YamlObject> &
> >       }
> > 
> >       std::optional<std::string> target = (*root)["target"].get<std::string>();
> > -     if (!target || *target != "bcm2835") {
> > +     if (target != "bcm2835") {
> >               LOG(RPI, Error) << "Unexpected target reported: expected \"bcm2835\", got "
> > -                             << *target;
> > +                             << (target ? target->c_str() : "(unknown)");
> >               return -EINVAL;
> >       }
> > 
> 
> -- 
> Regards,
> 
> Laurent Pinchart

Hi


2025. 04. 01. 14:56 keltezéssel, Kieran Bingham írta:
> Hi Naush,
> 
> A relatively small patch for RPi here, LGTM, but are you happy with
> this as a fix?
> 
> Quoting Laurent Pinchart (2025-03-26 02:00:19)
>> Hi Barnabás,
>>
>> Thank you for the patch.
>>
>> On Tue, Mar 25, 2025 at 03:07:45PM +0100, Barnabás Pőcze wrote:
>>> If `!target`, then `*target` is undefined behaviour, so check if the optional
>>> is empty when printing the error message. Simplify the check as well.
>>>
> 
> Any fixes tag suitable? (would make this list in the fixes log of the
> libcamera release)

I have sent a new version with the tags I believe are appropriate.
And I have also discovered that pisp has the same issue, so I fixed
that too in the version.


Regards,
Barnabás Pőcze

> 
> --
> Kieran
> 
> 
>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
>>
>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>
>>> ---
>>>   src/libcamera/pipeline/rpi/vc4/vc4.cpp | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
>>> index fd8d84b14..fe910bdf2 100644
>>> --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp
>>> +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
>>> @@ -510,9 +510,9 @@ int Vc4CameraData::platformPipelineConfigure(const std::unique_ptr<YamlObject> &
>>>        }
>>>
>>>        std::optional<std::string> target = (*root)["target"].get<std::string>();
>>> -     if (!target || *target != "bcm2835") {
>>> +     if (target != "bcm2835") {
>>>                LOG(RPI, Error) << "Unexpected target reported: expected \"bcm2835\", got "
>>> -                             << *target;
>>> +                             << (target ? target->c_str() : "(unknown)");
>>>                return -EINVAL;
>>>        }
>>>
>>
>> -- 
>> Regards,
>>
>> Laurent Pinchart