[libcamera-devel] test: buffer_import: Initialize media_ and video_

1975 diff mbox series

When the vivid module used to test buffer importing is not loaded, the
test correctly bails out, but during the clean up procedure tries to
access media_ and video_ fields, which, if not correctly initialized to
nullptr might retain random values and cause a segfault.

Fix this by initializing media_ and video_ to nullptr to make sure they
get ignored when cleaup() is called before they get initialized.

Fixes: e1a5873701a9 ("test: camera: Add buffer import and mapping test")
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 test/camera/buffer_import.cpp | 5 +++++
 1 file changed, 5 insertions(+)

Hi Jacopo,

Thank you for the patch.

On Mon, Sep 16, 2019 at 02:22:38PM +0200, Jacopo Mondi wrote:
> When the vivid module used to test buffer importing is not loaded, the
> test correctly bails out, but during the clean up procedure tries to
> access media_ and video_ fields, which, if not correctly initialized to
> nullptr might retain random values and cause a segfault.

s/might retain/will contain/

> Fix this by initializing media_ and video_ to nullptr to make sure they
> get ignored when cleaup() is called before they get initialized.

s/cleaup/cleanup/

Technically they will now be initialised before cleanup() is called ;-)

"... to make sure they are properly handled in cleanup()." ?

> Fixes: e1a5873701a9 ("test: camera: Add buffer import and mapping test")
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

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

> ---
>  test/camera/buffer_import.cpp | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp
> index 9364e3d1bc44..31af8336dc8a 100644
> --- a/test/camera/buffer_import.cpp
> +++ b/test/camera/buffer_import.cpp
> @@ -28,6 +28,11 @@ static constexpr unsigned int CAMERA_BUFFER_COUNT = 4;
>  class FrameSink
>  {
>  public:
> +	FrameSink()
> +		: media_(nullptr), video_(nullptr)
> +	{
> +	}
> +
>  	int init()
>  	{
>  		int ret;

Hi Jacopo,

On Mon, Sep 16, 2019 at 03:31:14PM +0300, Laurent Pinchart wrote:
> On Mon, Sep 16, 2019 at 02:22:38PM +0200, Jacopo Mondi wrote:
> > When the vivid module used to test buffer importing is not loaded, the
> > test correctly bails out, but during the clean up procedure tries to
> > access media_ and video_ fields, which, if not correctly initialized to
> > nullptr might retain random values and cause a segfault.
> 
> s/might retain/will contain/
> 
> > Fix this by initializing media_ and video_ to nullptr to make sure they
> > get ignored when cleaup() is called before they get initialized.
> 
> s/cleaup/cleanup/
> 
> Technically they will now be initialised before cleanup() is called ;-)
> 
> "... to make sure they are properly handled in cleanup()." ?
> 
> > Fixes: e1a5873701a9 ("test: camera: Add buffer import and mapping test")
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Actually isn't this needed only for video_ ? media_ is a
std::shared_ptr, the default constructor should create an empty
shared_ptr.

> > ---
> >  test/camera/buffer_import.cpp | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp
> > index 9364e3d1bc44..31af8336dc8a 100644
> > --- a/test/camera/buffer_import.cpp
> > +++ b/test/camera/buffer_import.cpp
> > @@ -28,6 +28,11 @@ static constexpr unsigned int CAMERA_BUFFER_COUNT = 4;
> >  class FrameSink
> >  {
> >  public:
> > +	FrameSink()
> > +		: media_(nullptr), video_(nullptr)
> > +	{
> > +	}
> > +
> >  	int init()
> >  	{
> >  		int ret;
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel