On Tue, 16 Jul 2024 at 14:30, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote: > > On Tue, Jul 16, 2024 at 10:48:20AM +0200, Jacopo Mondi wrote: > > > I did consider this, but did not like the undesired behavior it would > > > introduce. With your suggested change above, we now switch the color > > > space to an unsupported value (V4L2_COLORSPACE_DCI_P3, assuming we > > > shift by V4L2_COLORSPACE_LAST - 1) if the user passed an invalid > > > value. Of course, this does subsequently get fixed when used in > > > pispbe_try_format(), but not for the usage in pisp_be_formats.h. In > > > my original change, we revert to the default for the requested format > > > instead. Although, perhaps my test should be if > > > (!v4l2_is_colorspace_valid(f->fmt.pix_mp.colorspace) ... instead. > > > > Keep in mind the core 'sanitizes' the colorspace for you (see the > > usage of v4l_sanitize_colorspace() in > > drivers/media/v4l2-core/v4l2-ioctl.c) so a fix is need only to silence > > smatch not to actually address any run-time issue. > > > > Ah. That's true. Let's just ignore this warning in that case... > > Smatch would have an easier time parsing these if we passed p instead of > arg to return ops->vidioc_try_fmt_vid_cap_mplane(file, fh, arg) in > v4l_try_fmt(). > > struct v4l2_format *p = arg; > > v4l_sanitize_format(p); > > return ops->vidioc_try_fmt_vid_cap_mplane(file, fh, arg); > > I can just filter out that call to say that when v4l_try_fmt() calls > ops->vidioc_try_fmt_vid_cap_mplane() "arg" variable is trusted data. Sounds good, I'll drop this patch then. Naush
