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. regards, dan carpenter
