| Message ID | 20210218134752.1303582-1-kieran.bingham@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | 8201093830845f1ce420b8ca1dc550cd5a421b26 |
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran, On Thu, Feb 18, 2021 at 01:47:52PM +0000, Kieran Bingham wrote: > The call to setFormat uses uninitialised data, which whilst > not necessary a fault while setting, could cause unwanted effects. > > It is also trapped and reported by valgrind. > > Initialise the V4L2SubdeviceFormat structure correctly before use. ^ double space :) > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Indeed that could cause the sensorFormat to have an invalid mbus_code field which could trigger unwanted behaviour in SUBDEV_S_FMT Acked-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > --- > src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 3e6b88af4188..e2353e890e0f 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -846,7 +846,7 @@ int PipelineHandlerIPU3::initControls(IPU3CameraData *data) > */ > > /* Re-fetch the sensor info updated to use the largest resolution. */ > - V4L2SubdeviceFormat sensorFormat; > + V4L2SubdeviceFormat sensorFormat = {}; > sensorFormat.size = sensor->resolution(); > ret = sensor->setFormat(&sensorFormat); > if (ret) > -- > 2.25.1 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
On 18/02/2021 13:53, Jacopo Mondi wrote: > Hi Kieran, > > On Thu, Feb 18, 2021 at 01:47:52PM +0000, Kieran Bingham wrote: >> The call to setFormat uses uninitialised data, which whilst >> not necessary a fault while setting, could cause unwanted effects. >> >> It is also trapped and reported by valgrind. >> >> Initialise the V4L2SubdeviceFormat structure correctly before use. > ^ double space :) > >> >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > Indeed that could cause the sensorFormat to have an invalid mbus_code > field which could trigger unwanted behaviour in SUBDEV_S_FMT Aha, then I'll remove the "Whilst not necessarily a fault while setting" statement as it's incorrect. Thanks. > > Acked-by: Jacopo Mondi <jacopo@jmondi.org> > > Thanks > j > >> --- >> src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp >> index 3e6b88af4188..e2353e890e0f 100644 >> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp >> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp >> @@ -846,7 +846,7 @@ int PipelineHandlerIPU3::initControls(IPU3CameraData *data) >> */ >> >> /* Re-fetch the sensor info updated to use the largest resolution. */ >> - V4L2SubdeviceFormat sensorFormat; >> + V4L2SubdeviceFormat sensorFormat = {}; >> sensorFormat.size = sensor->resolution(); >> ret = sensor->setFormat(&sensorFormat); >> if (ret) >> -- >> 2.25.1 >> >> _______________________________________________ >> libcamera-devel mailing list >> libcamera-devel@lists.libcamera.org >> https://lists.libcamera.org/listinfo/libcamera-devel
Hi Kiera, On Thu, Feb 18, 2021 at 01:47:52PM +0000, Kieran Bingham wrote: > The call to setFormat uses uninitialised data, which whilst > not necessary a fault while setting, could cause unwanted effects. > > It is also trapped and reported by valgrind. > > Initialise the V4L2SubdeviceFormat structure correctly before use. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> With the changes suggested by Jacopo, Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > --- > src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 3e6b88af4188..e2353e890e0f 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -846,7 +846,7 @@ int PipelineHandlerIPU3::initControls(IPU3CameraData *data) > */ > > /* Re-fetch the sensor info updated to use the largest resolution. */ > - V4L2SubdeviceFormat sensorFormat; > + V4L2SubdeviceFormat sensorFormat = {}; > sensorFormat.size = sensor->resolution(); > ret = sensor->setFormat(&sensorFormat); > if (ret) > -- > 2.25.1 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 3e6b88af4188..e2353e890e0f 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -846,7 +846,7 @@ int PipelineHandlerIPU3::initControls(IPU3CameraData *data) */ /* Re-fetch the sensor info updated to use the largest resolution. */ - V4L2SubdeviceFormat sensorFormat; + V4L2SubdeviceFormat sensorFormat = {}; sensorFormat.size = sensor->resolution(); ret = sensor->setFormat(&sensorFormat); if (ret)
The call to setFormat uses uninitialised data, which whilst not necessary a fault while setting, could cause unwanted effects. It is also trapped and reported by valgrind. Initialise the V4L2SubdeviceFormat structure correctly before use. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)