* [PATCH 0/3] media: Zero-initialize structures passed to subdev pad ops
@ 2023-02-15 16:50 Laurent Pinchart
2023-02-15 16:50 ` [PATCH 1/3] media: Zero-initialize all structures passed to subdev pad operations Laurent Pinchart
` (4 more replies)
0 siblings, 5 replies; 16+ messages in thread
From: Laurent Pinchart @ 2023-02-15 16:50 UTC (permalink / raw)
To: linux-media
Cc: Hans Verkuil, Sakari Ailus, Tomi Valkeinen, Yong Zhi, Bingbu Cao,
Dan Scally, Tianshu Qiu, Eugen Hristev, Robert Foss, Todor Tomov,
Kieran Bingham, Sylwester Nawrocki, Prabhakar Lad, Benoit Parrot,
Shuah Khan, Michael Krufky, Steve Longerbeam, Philipp Zabel,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni, kernel,
linux-imx
Hello,
This patch series fixes a (surprisingly large) number of drivers that
don't zero-initialize structures passed to subdev pad operations.
The rationale is explained in patch 1/3, which fixes the issue: while
this doesn't cause any immediate problem, it leaves reserved fields
uninitialized, and any future change of in-kernel APIs that make use of
some of the reserved fields may introduce hard to catch breakages.
Patches 2/3 and 3/3 are not strictly required to fix the problem, but
they address coding style consistency issues that bothered me when
developing 1/3.
Laurent Pinchart (3):
media: Zero-initialize all structures passed to subdev pad operations
media: Prefer designated initializers over memset for subdev pad ops
media: USe designated initializers for all subdev pad ops
drivers/media/pci/cobalt/cobalt-v4l2.c | 21 ++++++----
drivers/media/pci/intel/ipu3/ipu3-cio2-main.c | 1 +
.../platform/microchip/microchip-isc-base.c | 5 ++-
.../media/platform/qcom/camss/camss-video.c | 5 ++-
.../media/platform/renesas/vsp1/vsp1_drm.c | 23 ++++++-----
.../media/platform/renesas/vsp1/vsp1_entity.c | 11 +++--
.../media/platform/renesas/vsp1/vsp1_video.c | 5 ++-
.../samsung/exynos4-is/fimc-capture.c | 18 +++++----
.../samsung/exynos4-is/fimc-isp-video.c | 10 +++--
.../platform/samsung/exynos4-is/fimc-lite.c | 9 +++--
.../samsung/s3c-camif/camif-capture.c | 5 ++-
.../platform/samsung/s3c-camif/camif-core.c | 5 ++-
.../media/platform/ti/am437x/am437x-vpfe.c | 35 ++++++++--------
drivers/media/platform/ti/cal/cal-video.c | 37 +++++++++--------
drivers/media/platform/ti/omap3isp/ispccdc.c | 5 ++-
drivers/media/platform/ti/omap3isp/ispvideo.c | 20 ++++++----
drivers/media/platform/xilinx/xilinx-dma.c | 5 ++-
drivers/media/test-drivers/vimc/vimc-common.c | 8 ++--
drivers/media/usb/dvb-usb/cxusb-analog.c | 14 +++----
.../media/deprecated/atmel/atmel-isc-base.c | 5 ++-
drivers/staging/media/imx/imx-media-capture.c | 40 ++++++++++---------
drivers/staging/media/imx/imx-media-utils.c | 8 ++--
drivers/staging/media/omap4iss/iss_video.c | 16 ++++----
drivers/staging/media/tegra-video/vi.c | 10 +++--
24 files changed, 182 insertions(+), 139 deletions(-)
base-commit: 83e0f265aa8d0e37cc8e15d318b64da0ec03ff41
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/3] media: Zero-initialize all structures passed to subdev pad operations
2023-02-15 16:50 [PATCH 0/3] media: Zero-initialize structures passed to subdev pad ops Laurent Pinchart
@ 2023-02-15 16:50 ` Laurent Pinchart
2023-02-27 22:30 ` Shuah Khan
` (3 more replies)
2023-02-15 16:50 ` [PATCH 2/3] media: Prefer designated initializers over memset for subdev pad ops Laurent Pinchart
` (3 subsequent siblings)
4 siblings, 4 replies; 16+ messages in thread
From: Laurent Pinchart @ 2023-02-15 16:50 UTC (permalink / raw)
To: linux-media
Cc: Hans Verkuil, Sakari Ailus, Tomi Valkeinen, Yong Zhi, Bingbu Cao,
Dan Scally, Tianshu Qiu, Eugen Hristev, Robert Foss, Todor Tomov,
Kieran Bingham, Sylwester Nawrocki, Prabhakar Lad, Benoit Parrot,
Shuah Khan, Michael Krufky, Steve Longerbeam, Philipp Zabel,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni, kernel,
linux-imx
Several drivers call subdev pad operations, passing structures that are
not fully zeroed. While the drivers initialize the fields they care
about explicitly, this results in reserved fields having uninitialized
values. Future kernel API changes that make use of those fields thus
risk breaking proper driver operation in ways that could be hard to
detect.
To avoid this, make the code more robust by zero-initializing all the
structures passed to subdev pad operation. Maintain a consistent coding
style by preferring designated initializers (which zero-initialize all
the fields that are not specified) over memset() where possible, and
make variable declarations local to inner scopes where applicable. One
notable exception to this rule is in the ipu3 driver, where a memset()
is needed as the structure is not a local variable but a function
parameter provided by the caller.
Not all fields of those structures can be initialized when declaring the
variables, as the values for those fields are computed later in the
code. Initialize the 'which' field in all cases, and other fields when
the variable declaration is so close to the v4l2_subdev_call() call that
it keeps all the context easily visible when reading the code, to avoid
hindering readability.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/pci/cobalt/cobalt-v4l2.c | 16 ++++++----
drivers/media/pci/intel/ipu3/ipu3-cio2-main.c | 1 +
.../media/platform/qcom/camss/camss-video.c | 5 ++--
.../media/platform/renesas/vsp1/vsp1_video.c | 5 ++--
.../samsung/exynos4-is/fimc-capture.c | 11 ++++---
.../samsung/exynos4-is/fimc-isp-video.c | 10 +++++--
.../platform/samsung/exynos4-is/fimc-lite.c | 9 ++++--
.../samsung/s3c-camif/camif-capture.c | 5 ++--
.../platform/samsung/s3c-camif/camif-core.c | 5 ++--
.../media/platform/ti/am437x/am437x-vpfe.c | 20 +++++++------
drivers/media/platform/ti/cal/cal-video.c | 29 ++++++++++---------
drivers/media/platform/ti/omap3isp/ispccdc.c | 5 ++--
drivers/media/platform/ti/omap3isp/ispvideo.c | 20 ++++++++-----
drivers/media/platform/xilinx/xilinx-dma.c | 5 ++--
drivers/media/test-drivers/vimc/vimc-common.c | 8 ++---
drivers/staging/media/imx/imx-media-capture.c | 28 ++++++++++--------
drivers/staging/media/omap4iss/iss_video.c | 10 ++++---
drivers/staging/media/tegra-video/vi.c | 10 ++++---
18 files changed, 120 insertions(+), 82 deletions(-)
diff --git a/drivers/media/pci/cobalt/cobalt-v4l2.c b/drivers/media/pci/cobalt/cobalt-v4l2.c
index 0ff37496c9ab..284c0909a282 100644
--- a/drivers/media/pci/cobalt/cobalt-v4l2.c
+++ b/drivers/media/pci/cobalt/cobalt-v4l2.c
@@ -708,7 +708,6 @@ static int cobalt_g_fmt_vid_cap(struct file *file, void *priv_fh,
{
struct cobalt_stream *s = video_drvdata(file);
struct v4l2_pix_format *pix = &f->fmt.pix;
- struct v4l2_subdev_format sd_fmt;
pix->width = s->width;
pix->height = s->height;
@@ -718,8 +717,11 @@ static int cobalt_g_fmt_vid_cap(struct file *file, void *priv_fh,
if (s->input == 1) {
pix->colorspace = V4L2_COLORSPACE_SRGB;
} else {
- sd_fmt.pad = s->pad_source;
- sd_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
+ struct v4l2_subdev_format sd_fmt = {
+ .pad = s->pad_source,
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
+
v4l2_subdev_call(s->sd, pad, get_fmt, NULL, &sd_fmt);
v4l2_fill_pix_format(pix, &sd_fmt.format);
}
@@ -735,7 +737,6 @@ static int cobalt_try_fmt_vid_cap(struct file *file, void *priv_fh,
{
struct cobalt_stream *s = video_drvdata(file);
struct v4l2_pix_format *pix = &f->fmt.pix;
- struct v4l2_subdev_format sd_fmt;
/* Check for min (QCIF) and max (Full HD) size */
if ((pix->width < 176) || (pix->height < 144)) {
@@ -760,8 +761,11 @@ static int cobalt_try_fmt_vid_cap(struct file *file, void *priv_fh,
pix->height = 1080;
pix->colorspace = V4L2_COLORSPACE_SRGB;
} else {
- sd_fmt.pad = s->pad_source;
- sd_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
+ struct v4l2_subdev_format sd_fmt = {
+ .pad = s->pad_source,
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
+
v4l2_subdev_call(s->sd, pad, get_fmt, NULL, &sd_fmt);
v4l2_fill_pix_format(pix, &sd_fmt.format);
}
diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
index 3b76a9d0383a..3c84cb121632 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
@@ -1305,6 +1305,7 @@ static int cio2_subdev_link_validate_get_format(struct media_pad *pad,
struct v4l2_subdev *sd =
media_entity_to_v4l2_subdev(pad->entity);
+ memset(fmt, 0, sizeof(*fmt));
fmt->which = V4L2_SUBDEV_FORMAT_ACTIVE;
fmt->pad = pad->index;
return v4l2_subdev_call(sd, pad, get_fmt, NULL, fmt);
diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
index 41deda232e4a..65c87af24463 100644
--- a/drivers/media/platform/qcom/camss/camss-video.c
+++ b/drivers/media/platform/qcom/camss/camss-video.c
@@ -342,7 +342,9 @@ static struct v4l2_subdev *video_remote_subdev(struct camss_video *video,
static int video_get_subdev_format(struct camss_video *video,
struct v4l2_format *format)
{
- struct v4l2_subdev_format fmt;
+ struct v4l2_subdev_format fmt = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
struct v4l2_subdev *subdev;
u32 pad;
int ret;
@@ -352,7 +354,6 @@ static int video_get_subdev_format(struct camss_video *video,
return -EPIPE;
fmt.pad = pad;
- fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);
if (ret)
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c b/drivers/media/platform/renesas/vsp1/vsp1_video.c
index 9d24647c8f32..ee0b61a52327 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_video.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c
@@ -62,7 +62,9 @@ vsp1_video_remote_subdev(struct media_pad *local, u32 *pad)
static int vsp1_video_verify_format(struct vsp1_video *video)
{
- struct v4l2_subdev_format fmt;
+ struct v4l2_subdev_format fmt = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
struct v4l2_subdev *subdev;
int ret;
@@ -70,7 +72,6 @@ static int vsp1_video_verify_format(struct vsp1_video *video)
if (subdev == NULL)
return -EINVAL;
- fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);
if (ret < 0)
return ret == -ENOIOCTLCMD ? -EINVAL : ret;
diff --git a/drivers/media/platform/samsung/exynos4-is/fimc-capture.c b/drivers/media/platform/samsung/exynos4-is/fimc-capture.c
index e3b95a2b7e04..4800751a401c 100644
--- a/drivers/media/platform/samsung/exynos4-is/fimc-capture.c
+++ b/drivers/media/platform/samsung/exynos4-is/fimc-capture.c
@@ -854,7 +854,7 @@ static int fimc_get_sensor_frame_desc(struct v4l2_subdev *sensor,
struct v4l2_plane_pix_format *plane_fmt,
unsigned int num_planes, bool try)
{
- struct v4l2_mbus_frame_desc fd;
+ struct v4l2_mbus_frame_desc fd = { };
int i, ret;
int pad;
@@ -1095,7 +1095,12 @@ static int fimc_cap_g_input(struct file *file, void *priv, unsigned int *i)
*/
static int fimc_pipeline_validate(struct fimc_dev *fimc)
{
- struct v4l2_subdev_format sink_fmt, src_fmt;
+ struct v4l2_subdev_format sink_fmt = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
+ struct v4l2_subdev_format src_fmt = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
struct fimc_vid_cap *vc = &fimc->vid_cap;
struct v4l2_subdev *sd = &vc->subdev;
struct fimc_pipeline *p = to_fimc_pipeline(vc->ve.pipe);
@@ -1132,7 +1137,6 @@ static int fimc_pipeline_validate(struct fimc_dev *fimc)
sink_fmt.format.code = ff->fmt ? ff->fmt->mbus_code : 0;
} else {
sink_fmt.pad = sink_pad->index;
- sink_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &sink_fmt);
if (ret < 0 && ret != -ENOIOCTLCMD)
return -EPIPE;
@@ -1141,7 +1145,6 @@ static int fimc_pipeline_validate(struct fimc_dev *fimc)
/* Retrieve format at the source pad */
sd = media_entity_to_v4l2_subdev(src_pad->entity);
src_fmt.pad = src_pad->index;
- src_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &src_fmt);
if (ret < 0 && ret != -ENOIOCTLCMD)
return -EPIPE;
diff --git a/drivers/media/platform/samsung/exynos4-is/fimc-isp-video.c b/drivers/media/platform/samsung/exynos4-is/fimc-isp-video.c
index f6a302fa8d37..8fa26969c411 100644
--- a/drivers/media/platform/samsung/exynos4-is/fimc-isp-video.c
+++ b/drivers/media/platform/samsung/exynos4-is/fimc-isp-video.c
@@ -449,17 +449,22 @@ static int isp_video_s_fmt_mplane(struct file *file, void *priv,
static int isp_video_pipeline_validate(struct fimc_isp *isp)
{
struct v4l2_subdev *sd = &isp->subdev;
- struct v4l2_subdev_format sink_fmt, src_fmt;
struct media_pad *pad;
int ret;
while (1) {
+ struct v4l2_subdev_format sink_fmt = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
+ struct v4l2_subdev_format src_fmt = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
+
/* Retrieve format at the sink pad */
pad = &sd->entity.pads[0];
if (!(pad->flags & MEDIA_PAD_FL_SINK))
break;
sink_fmt.pad = pad->index;
- sink_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &sink_fmt);
if (ret < 0 && ret != -ENOIOCTLCMD)
return -EPIPE;
@@ -471,7 +476,6 @@ static int isp_video_pipeline_validate(struct fimc_isp *isp)
sd = media_entity_to_v4l2_subdev(pad->entity);
src_fmt.pad = pad->index;
- src_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &src_fmt);
if (ret < 0 && ret != -ENOIOCTLCMD)
return -EPIPE;
diff --git a/drivers/media/platform/samsung/exynos4-is/fimc-lite.c b/drivers/media/platform/samsung/exynos4-is/fimc-lite.c
index e185a40305a8..162e44efcb3e 100644
--- a/drivers/media/platform/samsung/exynos4-is/fimc-lite.c
+++ b/drivers/media/platform/samsung/exynos4-is/fimc-lite.c
@@ -765,7 +765,12 @@ static int fimc_lite_s_fmt_mplane(struct file *file, void *priv,
static int fimc_pipeline_validate(struct fimc_lite *fimc)
{
struct v4l2_subdev *sd = &fimc->subdev;
- struct v4l2_subdev_format sink_fmt, src_fmt;
+ struct v4l2_subdev_format sink_fmt = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
+ struct v4l2_subdev_format src_fmt = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
struct media_pad *pad;
int ret;
@@ -782,7 +787,6 @@ static int fimc_pipeline_validate(struct fimc_lite *fimc)
sink_fmt.format.code = fimc->inp_frame.fmt->mbus_code;
} else {
sink_fmt.pad = pad->index;
- sink_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(sd, pad, get_fmt, NULL,
&sink_fmt);
if (ret < 0 && ret != -ENOIOCTLCMD)
@@ -795,7 +799,6 @@ static int fimc_pipeline_validate(struct fimc_lite *fimc)
sd = media_entity_to_v4l2_subdev(pad->entity);
src_fmt.pad = pad->index;
- src_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &src_fmt);
if (ret < 0 && ret != -ENOIOCTLCMD)
return -EPIPE;
diff --git a/drivers/media/platform/samsung/s3c-camif/camif-capture.c b/drivers/media/platform/samsung/s3c-camif/camif-capture.c
index db106ebdf870..76634d242b10 100644
--- a/drivers/media/platform/samsung/s3c-camif/camif-capture.c
+++ b/drivers/media/platform/samsung/s3c-camif/camif-capture.c
@@ -806,7 +806,9 @@ static int s3c_camif_vidioc_s_fmt(struct file *file, void *priv,
/* Only check pixel formats at the sensor and the camif subdev pads */
static int camif_pipeline_validate(struct camif_dev *camif)
{
- struct v4l2_subdev_format src_fmt;
+ struct v4l2_subdev_format src_fmt = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
struct media_pad *pad;
int ret;
@@ -816,7 +818,6 @@ static int camif_pipeline_validate(struct camif_dev *camif)
return -EPIPE;
src_fmt.pad = pad->index;
- src_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(camif->sensor.sd, pad, get_fmt, NULL, &src_fmt);
if (ret < 0 && ret != -ENOIOCTLCMD)
return -EPIPE;
diff --git a/drivers/media/platform/samsung/s3c-camif/camif-core.c b/drivers/media/platform/samsung/s3c-camif/camif-core.c
index 6e8ef86566b7..9827510b7b50 100644
--- a/drivers/media/platform/samsung/s3c-camif/camif-core.c
+++ b/drivers/media/platform/samsung/s3c-camif/camif-core.c
@@ -190,7 +190,9 @@ static int camif_register_sensor(struct camif_dev *camif)
struct s3c_camif_sensor_info *sensor = &camif->pdata.sensor;
struct v4l2_device *v4l2_dev = &camif->v4l2_dev;
struct i2c_adapter *adapter;
- struct v4l2_subdev_format format;
+ struct v4l2_subdev_format format = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
struct v4l2_subdev *sd;
int ret;
@@ -220,7 +222,6 @@ static int camif_register_sensor(struct camif_dev *camif)
/* Get initial pixel format and set it at the camif sink pad */
format.pad = 0;
- format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &format);
if (ret < 0)
diff --git a/drivers/media/platform/ti/am437x/am437x-vpfe.c b/drivers/media/platform/ti/am437x/am437x-vpfe.c
index 2dfae9bc0bba..f23085b5954b 100644
--- a/drivers/media/platform/ti/am437x/am437x-vpfe.c
+++ b/drivers/media/platform/ti/am437x/am437x-vpfe.c
@@ -1285,13 +1285,13 @@ static int __subdev_get_format(struct vpfe_device *vpfe,
struct v4l2_mbus_framefmt *fmt)
{
struct v4l2_subdev *sd = vpfe->current_subdev->sd;
- struct v4l2_subdev_format sd_fmt;
+ struct v4l2_subdev_format sd_fmt = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ .pad = 0,
+ };
struct v4l2_mbus_framefmt *mbus_fmt = &sd_fmt.format;
int ret;
- sd_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
- sd_fmt.pad = 0;
-
ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &sd_fmt);
if (ret)
return ret;
@@ -1309,12 +1309,13 @@ static int __subdev_set_format(struct vpfe_device *vpfe,
struct v4l2_mbus_framefmt *fmt)
{
struct v4l2_subdev *sd = vpfe->current_subdev->sd;
- struct v4l2_subdev_format sd_fmt;
+ struct v4l2_subdev_format sd_fmt = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ .pad = 0,
+ };
struct v4l2_mbus_framefmt *mbus_fmt = &sd_fmt.format;
int ret;
- sd_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
- sd_fmt.pad = 0;
*mbus_fmt = *fmt;
ret = v4l2_subdev_call(sd, pad, set_fmt, NULL, &sd_fmt);
@@ -1393,7 +1394,9 @@ static int vpfe_try_fmt(struct file *file, void *priv,
struct vpfe_device *vpfe = video_drvdata(file);
struct v4l2_subdev *sd = vpfe->current_subdev->sd;
const struct vpfe_fmt *fmt;
- struct v4l2_subdev_frame_size_enum fse;
+ struct v4l2_subdev_frame_size_enum fse = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
int ret, found;
fmt = find_format_by_pix(vpfe, f->fmt.pix.pixelformat);
@@ -1412,7 +1415,6 @@ static int vpfe_try_fmt(struct file *file, void *priv,
found = false;
fse.pad = 0;
fse.code = fmt->code;
- fse.which = V4L2_SUBDEV_FORMAT_ACTIVE;
for (fse.index = 0; ; fse.index++) {
ret = v4l2_subdev_call(sd, pad, enum_frame_size,
NULL, &fse);
diff --git a/drivers/media/platform/ti/cal/cal-video.c b/drivers/media/platform/ti/cal/cal-video.c
index 4eade409d5d3..51ed82172d71 100644
--- a/drivers/media/platform/ti/cal/cal-video.c
+++ b/drivers/media/platform/ti/cal/cal-video.c
@@ -117,13 +117,13 @@ static int cal_legacy_enum_fmt_vid_cap(struct file *file, void *priv,
static int __subdev_get_format(struct cal_ctx *ctx,
struct v4l2_mbus_framefmt *fmt)
{
- struct v4l2_subdev_format sd_fmt;
+ struct v4l2_subdev_format sd_fmt = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ .pad = 0,
+ };
struct v4l2_mbus_framefmt *mbus_fmt = &sd_fmt.format;
int ret;
- sd_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
- sd_fmt.pad = 0;
-
ret = v4l2_subdev_call(ctx->phy->source, pad, get_fmt, NULL, &sd_fmt);
if (ret)
return ret;
@@ -139,12 +139,13 @@ static int __subdev_get_format(struct cal_ctx *ctx,
static int __subdev_set_format(struct cal_ctx *ctx,
struct v4l2_mbus_framefmt *fmt)
{
- struct v4l2_subdev_format sd_fmt;
+ struct v4l2_subdev_format sd_fmt = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ .pad = 0,
+ };
struct v4l2_mbus_framefmt *mbus_fmt = &sd_fmt.format;
int ret;
- sd_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
- sd_fmt.pad = 0;
*mbus_fmt = *fmt;
ret = v4l2_subdev_call(ctx->phy->source, pad, set_fmt, NULL, &sd_fmt);
@@ -190,7 +191,9 @@ static int cal_legacy_try_fmt_vid_cap(struct file *file, void *priv,
{
struct cal_ctx *ctx = video_drvdata(file);
const struct cal_format_info *fmtinfo;
- struct v4l2_subdev_frame_size_enum fse;
+ struct v4l2_subdev_frame_size_enum fse = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
int found;
fmtinfo = find_format_by_pix(ctx, f->fmt.pix.pixelformat);
@@ -209,7 +212,6 @@ static int cal_legacy_try_fmt_vid_cap(struct file *file, void *priv,
found = false;
fse.pad = 0;
fse.code = fmtinfo->code;
- fse.which = V4L2_SUBDEV_FORMAT_ACTIVE;
for (fse.index = 0; ; fse.index++) {
int ret;
@@ -302,7 +304,11 @@ static int cal_legacy_enum_framesizes(struct file *file, void *fh,
{
struct cal_ctx *ctx = video_drvdata(file);
const struct cal_format_info *fmtinfo;
- struct v4l2_subdev_frame_size_enum fse;
+ struct v4l2_subdev_frame_size_enum fse = {
+ .index = fsize->index,
+ .pad = 0,
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
int ret;
/* check for valid format */
@@ -313,10 +319,7 @@ static int cal_legacy_enum_framesizes(struct file *file, void *fh,
return -EINVAL;
}
- fse.index = fsize->index;
- fse.pad = 0;
fse.code = fmtinfo->code;
- fse.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(ctx->phy->source, pad, enum_frame_size, NULL,
&fse);
diff --git a/drivers/media/platform/ti/omap3isp/ispccdc.c b/drivers/media/platform/ti/omap3isp/ispccdc.c
index 11afb8aec292..fdcdffe5fecb 100644
--- a/drivers/media/platform/ti/omap3isp/ispccdc.c
+++ b/drivers/media/platform/ti/omap3isp/ispccdc.c
@@ -1118,7 +1118,9 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
struct v4l2_mbus_framefmt *format;
const struct v4l2_rect *crop;
const struct isp_format_info *fmt_info;
- struct v4l2_subdev_format fmt_src;
+ struct v4l2_subdev_format fmt_src = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
unsigned int depth_out;
unsigned int depth_in = 0;
struct media_pad *pad;
@@ -1150,7 +1152,6 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
* input format is a non-BT.656 YUV variant.
*/
fmt_src.pad = pad->index;
- fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
if (!v4l2_subdev_call(sensor, pad, get_fmt, NULL, &fmt_src)) {
fmt_info = omap3isp_video_format_info(fmt_src.format.code);
depth_in = fmt_info->width;
diff --git a/drivers/media/platform/ti/omap3isp/ispvideo.c b/drivers/media/platform/ti/omap3isp/ispvideo.c
index ddc7d08d4f96..daca689dc082 100644
--- a/drivers/media/platform/ti/omap3isp/ispvideo.c
+++ b/drivers/media/platform/ti/omap3isp/ispvideo.c
@@ -268,7 +268,9 @@ static int isp_video_get_graph_data(struct isp_video *video,
static int
__isp_video_get_format(struct isp_video *video, struct v4l2_format *format)
{
- struct v4l2_subdev_format fmt;
+ struct v4l2_subdev_format fmt = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
struct v4l2_subdev *subdev;
u32 pad;
int ret;
@@ -278,7 +280,6 @@ __isp_video_get_format(struct isp_video *video, struct v4l2_format *format)
return -EINVAL;
fmt.pad = pad;
- fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
mutex_lock(&video->mutex);
ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);
@@ -731,7 +732,9 @@ static int
isp_video_try_format(struct file *file, void *fh, struct v4l2_format *format)
{
struct isp_video *video = video_drvdata(file);
- struct v4l2_subdev_format fmt;
+ struct v4l2_subdev_format fmt = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
struct v4l2_subdev *subdev;
u32 pad;
int ret;
@@ -746,7 +749,6 @@ isp_video_try_format(struct file *file, void *fh, struct v4l2_format *format)
isp_video_pix_to_mbus(&format->fmt.pix, &fmt.format);
fmt.pad = pad;
- fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);
if (ret)
return ret == -ENOIOCTLCMD ? -ENOTTY : ret;
@@ -759,7 +761,9 @@ static int
isp_video_get_selection(struct file *file, void *fh, struct v4l2_selection *sel)
{
struct isp_video *video = video_drvdata(file);
- struct v4l2_subdev_format format;
+ struct v4l2_subdev_format format = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
struct v4l2_subdev *subdev;
struct v4l2_subdev_selection sdsel = {
.which = V4L2_SUBDEV_FORMAT_ACTIVE,
@@ -799,7 +803,6 @@ isp_video_get_selection(struct file *file, void *fh, struct v4l2_selection *sel)
return ret;
format.pad = pad;
- format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &format);
if (ret < 0)
return ret == -ENOIOCTLCMD ? -ENOTTY : ret;
@@ -957,7 +960,9 @@ static int isp_video_check_external_subdevs(struct isp_video *video,
struct media_pad *source_pad;
struct media_entity *source = NULL;
struct media_entity *sink;
- struct v4l2_subdev_format fmt;
+ struct v4l2_subdev_format fmt = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
struct v4l2_ext_controls ctrls;
struct v4l2_ext_control ctrl;
unsigned int i;
@@ -993,7 +998,6 @@ static int isp_video_check_external_subdevs(struct isp_video *video,
pipe->external = media_entity_to_v4l2_subdev(source);
fmt.pad = source_pad->index;
- fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(media_entity_to_v4l2_subdev(sink),
pad, get_fmt, NULL, &fmt);
if (unlikely(ret < 0)) {
diff --git a/drivers/media/platform/xilinx/xilinx-dma.c b/drivers/media/platform/xilinx/xilinx-dma.c
index fee02c8c85fd..80d6f5b072ea 100644
--- a/drivers/media/platform/xilinx/xilinx-dma.c
+++ b/drivers/media/platform/xilinx/xilinx-dma.c
@@ -56,7 +56,9 @@ xvip_dma_remote_subdev(struct media_pad *local, u32 *pad)
static int xvip_dma_verify_format(struct xvip_dma *dma)
{
- struct v4l2_subdev_format fmt;
+ struct v4l2_subdev_format fmt = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
struct v4l2_subdev *subdev;
int ret;
@@ -64,7 +66,6 @@ static int xvip_dma_verify_format(struct xvip_dma *dma)
if (subdev == NULL)
return -EPIPE;
- fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);
if (ret < 0)
return ret == -ENOIOCTLCMD ? -EINVAL : ret;
diff --git a/drivers/media/test-drivers/vimc/vimc-common.c b/drivers/media/test-drivers/vimc/vimc-common.c
index 7b27153c0728..2e72974e35b4 100644
--- a/drivers/media/test-drivers/vimc/vimc-common.c
+++ b/drivers/media/test-drivers/vimc/vimc-common.c
@@ -241,13 +241,13 @@ static int vimc_get_pix_format(struct media_pad *pad,
if (is_media_entity_v4l2_subdev(pad->entity)) {
struct v4l2_subdev *sd =
media_entity_to_v4l2_subdev(pad->entity);
- struct v4l2_subdev_format sd_fmt;
+ struct v4l2_subdev_format sd_fmt = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ .pad = pad->index,
+ };
const struct vimc_pix_map *pix_map;
int ret;
- sd_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
- sd_fmt.pad = pad->index;
-
ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &sd_fmt);
if (ret)
return ret;
diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
index 93ba09236010..39666db59c4e 100644
--- a/drivers/staging/media/imx/imx-media-capture.c
+++ b/drivers/staging/media/imx/imx-media-capture.c
@@ -353,12 +353,13 @@ static int capture_legacy_enum_fmt_vid_cap(struct file *file, void *fh,
{
struct capture_priv *priv = video_drvdata(file);
const struct imx_media_pixfmt *cc_src;
- struct v4l2_subdev_format fmt_src;
+ struct v4l2_subdev_format fmt_src = {
+ .pad = priv->src_sd_pad,
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
u32 fourcc;
int ret;
- fmt_src.pad = priv->src_sd_pad;
- fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(priv->src_sd, pad, get_fmt, NULL, &fmt_src);
if (ret) {
dev_err(priv->dev, "failed to get src_sd format\n");
@@ -426,11 +427,12 @@ static int capture_legacy_try_fmt_vid_cap(struct file *file, void *fh,
struct v4l2_format *f)
{
struct capture_priv *priv = video_drvdata(file);
- struct v4l2_subdev_format fmt_src;
+ struct v4l2_subdev_format fmt_src = {
+ .pad = priv->src_sd_pad,
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
int ret;
- fmt_src.pad = priv->src_sd_pad;
- fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(priv->src_sd, pad, get_fmt, NULL, &fmt_src);
if (ret)
return ret;
@@ -445,7 +447,10 @@ static int capture_legacy_s_fmt_vid_cap(struct file *file, void *fh,
struct v4l2_format *f)
{
struct capture_priv *priv = video_drvdata(file);
- struct v4l2_subdev_format fmt_src;
+ struct v4l2_subdev_format fmt_src = {
+ .pad = priv->src_sd_pad,
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
const struct imx_media_pixfmt *cc;
int ret;
@@ -454,8 +459,6 @@ static int capture_legacy_s_fmt_vid_cap(struct file *file, void *fh,
return -EBUSY;
}
- fmt_src.pad = priv->src_sd_pad;
- fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(priv->src_sd, pad, get_fmt, NULL, &fmt_src);
if (ret)
return ret;
@@ -670,13 +673,14 @@ static void capture_buf_queue(struct vb2_buffer *vb)
static int capture_validate_fmt(struct capture_priv *priv)
{
- struct v4l2_subdev_format fmt_src;
+ struct v4l2_subdev_format fmt_src = {
+ .pad = priv->src_sd_pad,
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
const struct imx_media_pixfmt *cc;
int ret;
/* Retrieve the media bus format on the source subdev. */
- fmt_src.pad = priv->src_sd_pad;
- fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(priv->src_sd, pad, get_fmt, NULL, &fmt_src);
if (ret)
return ret;
diff --git a/drivers/staging/media/omap4iss/iss_video.c b/drivers/staging/media/omap4iss/iss_video.c
index 05548eab7daa..1406445a30c3 100644
--- a/drivers/staging/media/omap4iss/iss_video.c
+++ b/drivers/staging/media/omap4iss/iss_video.c
@@ -610,7 +610,9 @@ static int
iss_video_try_format(struct file *file, void *fh, struct v4l2_format *format)
{
struct iss_video *video = video_drvdata(file);
- struct v4l2_subdev_format fmt;
+ struct v4l2_subdev_format fmt = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
struct v4l2_subdev *subdev;
u32 pad;
int ret;
@@ -625,7 +627,6 @@ iss_video_try_format(struct file *file, void *fh, struct v4l2_format *format)
iss_video_pix_to_mbus(&format->fmt.pix, &fmt.format);
fmt.pad = pad;
- fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);
if (ret)
return ret;
@@ -638,7 +639,9 @@ static int
iss_video_get_selection(struct file *file, void *fh, struct v4l2_selection *sel)
{
struct iss_video *video = video_drvdata(file);
- struct v4l2_subdev_format format;
+ struct v4l2_subdev_format format = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
struct v4l2_subdev *subdev;
struct v4l2_subdev_selection sdsel = {
.which = V4L2_SUBDEV_FORMAT_ACTIVE,
@@ -679,7 +682,6 @@ iss_video_get_selection(struct file *file, void *fh, struct v4l2_selection *sel)
return ret;
format.pad = pad;
- format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &format);
if (ret < 0)
return ret == -ENOIOCTLCMD ? -ENOTTY : ret;
diff --git a/drivers/staging/media/tegra-video/vi.c b/drivers/staging/media/tegra-video/vi.c
index 11dd142c98c5..8504adff9919 100644
--- a/drivers/staging/media/tegra-video/vi.c
+++ b/drivers/staging/media/tegra-video/vi.c
@@ -493,7 +493,9 @@ static int __tegra_channel_try_format(struct tegra_vi_channel *chan,
const struct tegra_video_format *fmtinfo;
static struct lock_class_key key;
struct v4l2_subdev *subdev;
- struct v4l2_subdev_format fmt;
+ struct v4l2_subdev_format fmt = {
+ .which = V4L2_SUBDEV_FORMAT_TRY,
+ };
struct v4l2_subdev_state *sd_state;
struct v4l2_subdev_frame_size_enum fse = {
.which = V4L2_SUBDEV_FORMAT_TRY,
@@ -529,7 +531,6 @@ static int __tegra_channel_try_format(struct tegra_vi_channel *chan,
}
pix->field = V4L2_FIELD_NONE;
- fmt.which = V4L2_SUBDEV_FORMAT_TRY;
fmt.pad = 0;
v4l2_fill_mbus_format(&fmt.format, pix, fmtinfo->code);
@@ -590,7 +591,9 @@ static int tegra_channel_set_format(struct file *file, void *fh,
{
struct tegra_vi_channel *chan = video_drvdata(file);
const struct tegra_video_format *fmtinfo;
- struct v4l2_subdev_format fmt;
+ struct v4l2_subdev_format fmt = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
struct v4l2_subdev *subdev;
struct v4l2_pix_format *pix = &format->fmt.pix;
int ret;
@@ -605,7 +608,6 @@ static int tegra_channel_set_format(struct file *file, void *fh,
fmtinfo = tegra_get_format_by_fourcc(chan->vi, pix->pixelformat);
- fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
fmt.pad = 0;
v4l2_fill_mbus_format(&fmt.format, pix, fmtinfo->code);
subdev = tegra_channel_get_remote_source_subdev(chan);
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/3] media: Prefer designated initializers over memset for subdev pad ops
2023-02-15 16:50 [PATCH 0/3] media: Zero-initialize structures passed to subdev pad ops Laurent Pinchart
2023-02-15 16:50 ` [PATCH 1/3] media: Zero-initialize all structures passed to subdev pad operations Laurent Pinchart
@ 2023-02-15 16:50 ` Laurent Pinchart
2023-02-28 9:09 ` Lad, Prabhakar
2023-03-06 13:01 ` Philipp Zabel
2023-02-15 16:50 ` [PATCH 3/3] media: USe designated initializers for all " Laurent Pinchart
` (2 subsequent siblings)
4 siblings, 2 replies; 16+ messages in thread
From: Laurent Pinchart @ 2023-02-15 16:50 UTC (permalink / raw)
To: linux-media
Cc: Hans Verkuil, Sakari Ailus, Tomi Valkeinen, Yong Zhi, Bingbu Cao,
Dan Scally, Tianshu Qiu, Eugen Hristev, Robert Foss, Todor Tomov,
Kieran Bingham, Sylwester Nawrocki, Prabhakar Lad, Benoit Parrot,
Shuah Khan, Michael Krufky, Steve Longerbeam, Philipp Zabel,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni, kernel,
linux-imx
Structures passed to subdev pad operations are all zero-initialized, but
not always with the same kind of code constructs. While most drivers
used designated initializers, which zero all the fields that are not
specified, when declaring variables, some use memset(). Those two
methods lead to the same end result, and, depending on compiler
optimizations, may even be completely equivalent, but they're not
consistent.
Improve coding style consistency by using designated initializers
instead of calling memset(). Where applicable, also move the variables
to inner scopes of for loops to ensure correct initialization in all
iterations.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/platform/renesas/vsp1/vsp1_drm.c | 18 +++++++++---------
.../media/platform/renesas/vsp1/vsp1_entity.c | 11 +++++------
.../platform/samsung/exynos4-is/fimc-capture.c | 7 ++++---
drivers/media/platform/ti/am437x/am437x-vpfe.c | 15 ++++++++-------
drivers/media/platform/ti/cal/cal-video.c | 8 ++++----
drivers/media/usb/dvb-usb/cxusb-analog.c | 14 +++++++-------
drivers/staging/media/imx/imx-media-capture.c | 12 ++++++------
drivers/staging/media/imx/imx-media-utils.c | 8 ++++----
drivers/staging/media/omap4iss/iss_video.c | 6 +++---
9 files changed, 50 insertions(+), 49 deletions(-)
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drm.c b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
index c6f25200982c..7fe375b6322c 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
@@ -66,7 +66,9 @@ static int vsp1_du_insert_uif(struct vsp1_device *vsp1,
struct vsp1_entity *prev, unsigned int prev_pad,
struct vsp1_entity *next, unsigned int next_pad)
{
- struct v4l2_subdev_format format;
+ struct v4l2_subdev_format format = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
int ret;
if (!uif) {
@@ -82,8 +84,6 @@ static int vsp1_du_insert_uif(struct vsp1_device *vsp1,
prev->sink = uif;
prev->sink_pad = UIF_PAD_SINK;
- memset(&format, 0, sizeof(format));
- format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
format.pad = prev_pad;
ret = v4l2_subdev_call(&prev->subdev, pad, get_fmt, NULL, &format);
@@ -118,8 +118,12 @@ static int vsp1_du_pipeline_setup_rpf(struct vsp1_device *vsp1,
struct vsp1_entity *uif,
unsigned int brx_input)
{
- struct v4l2_subdev_selection sel;
- struct v4l2_subdev_format format;
+ struct v4l2_subdev_selection sel = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
+ struct v4l2_subdev_format format = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
const struct v4l2_rect *crop;
int ret;
@@ -129,8 +133,6 @@ static int vsp1_du_pipeline_setup_rpf(struct vsp1_device *vsp1,
*/
crop = &vsp1->drm->inputs[rpf->entity.index].crop;
- memset(&format, 0, sizeof(format));
- format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
format.pad = RWPF_PAD_SINK;
format.format.width = crop->width + crop->left;
format.format.height = crop->height + crop->top;
@@ -147,8 +149,6 @@ static int vsp1_du_pipeline_setup_rpf(struct vsp1_device *vsp1,
__func__, format.format.width, format.format.height,
format.format.code, rpf->entity.index);
- memset(&sel, 0, sizeof(sel));
- sel.which = V4L2_SUBDEV_FORMAT_ACTIVE;
sel.pad = RWPF_PAD_SINK;
sel.target = V4L2_SEL_TGT_CROP;
sel.r = *crop;
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.c b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
index 4c3bd2b1ca28..c31f05a80bb5 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
@@ -184,15 +184,14 @@ vsp1_entity_get_pad_selection(struct vsp1_entity *entity,
int vsp1_entity_init_cfg(struct v4l2_subdev *subdev,
struct v4l2_subdev_state *sd_state)
{
- struct v4l2_subdev_format format;
unsigned int pad;
for (pad = 0; pad < subdev->entity.num_pads - 1; ++pad) {
- memset(&format, 0, sizeof(format));
-
- format.pad = pad;
- format.which = sd_state ? V4L2_SUBDEV_FORMAT_TRY
- : V4L2_SUBDEV_FORMAT_ACTIVE;
+ struct v4l2_subdev_format format = {
+ .pad = pad,
+ .which = sd_state ? V4L2_SUBDEV_FORMAT_TRY
+ : V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
v4l2_subdev_call(subdev, pad, set_fmt, sd_state, &format);
}
diff --git a/drivers/media/platform/samsung/exynos4-is/fimc-capture.c b/drivers/media/platform/samsung/exynos4-is/fimc-capture.c
index 4800751a401c..a0d43bf892e6 100644
--- a/drivers/media/platform/samsung/exynos4-is/fimc-capture.c
+++ b/drivers/media/platform/samsung/exynos4-is/fimc-capture.c
@@ -763,7 +763,10 @@ static int fimc_pipeline_try_format(struct fimc_ctx *ctx,
struct fimc_dev *fimc = ctx->fimc_dev;
struct fimc_pipeline *p = to_fimc_pipeline(fimc->vid_cap.ve.pipe);
struct v4l2_subdev *sd = p->subdevs[IDX_SENSOR];
- struct v4l2_subdev_format sfmt;
+ struct v4l2_subdev_format sfmt = {
+ .which = set ? V4L2_SUBDEV_FORMAT_ACTIVE
+ : V4L2_SUBDEV_FORMAT_TRY,
+ };
struct v4l2_mbus_framefmt *mf = &sfmt.format;
struct media_entity *me;
struct fimc_fmt *ffmt;
@@ -774,9 +777,7 @@ static int fimc_pipeline_try_format(struct fimc_ctx *ctx,
if (WARN_ON(!sd || !tfmt))
return -EINVAL;
- memset(&sfmt, 0, sizeof(sfmt));
sfmt.format = *tfmt;
- sfmt.which = set ? V4L2_SUBDEV_FORMAT_ACTIVE : V4L2_SUBDEV_FORMAT_TRY;
me = fimc_pipeline_get_head(&sd->entity);
diff --git a/drivers/media/platform/ti/am437x/am437x-vpfe.c b/drivers/media/platform/ti/am437x/am437x-vpfe.c
index f23085b5954b..9fcbf2a2b7c3 100644
--- a/drivers/media/platform/ti/am437x/am437x-vpfe.c
+++ b/drivers/media/platform/ti/am437x/am437x-vpfe.c
@@ -1501,7 +1501,9 @@ static int vpfe_enum_size(struct file *file, void *priv,
struct v4l2_frmsizeenum *fsize)
{
struct vpfe_device *vpfe = video_drvdata(file);
- struct v4l2_subdev_frame_size_enum fse;
+ struct v4l2_subdev_frame_size_enum fse = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
struct v4l2_subdev *sd = vpfe->current_subdev->sd;
struct vpfe_fmt *fmt;
int ret;
@@ -1516,11 +1518,9 @@ static int vpfe_enum_size(struct file *file, void *priv,
memset(fsize->reserved, 0x0, sizeof(fsize->reserved));
- memset(&fse, 0x0, sizeof(fse));
fse.index = fsize->index;
fse.pad = 0;
fse.code = fmt->code;
- fse.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(sd, pad, enum_frame_size, NULL, &fse);
if (ret)
return ret;
@@ -2148,7 +2148,6 @@ vpfe_async_bound(struct v4l2_async_notifier *notifier,
{
struct vpfe_device *vpfe = container_of(notifier->v4l2_dev,
struct vpfe_device, v4l2_dev);
- struct v4l2_subdev_mbus_code_enum mbus_code;
struct vpfe_subdev_info *sdinfo;
struct vpfe_fmt *fmt;
int ret = 0;
@@ -2175,9 +2174,11 @@ vpfe_async_bound(struct v4l2_async_notifier *notifier,
vpfe->num_active_fmt = 0;
for (j = 0, i = 0; (ret != -EINVAL); ++j) {
- memset(&mbus_code, 0, sizeof(mbus_code));
- mbus_code.index = j;
- mbus_code.which = V4L2_SUBDEV_FORMAT_ACTIVE;
+ struct v4l2_subdev_mbus_code_enum mbus_code = {
+ .index = j,
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
+
ret = v4l2_subdev_call(subdev, pad, enum_mbus_code,
NULL, &mbus_code);
if (ret)
diff --git a/drivers/media/platform/ti/cal/cal-video.c b/drivers/media/platform/ti/cal/cal-video.c
index 51ed82172d71..ca906a9e4222 100644
--- a/drivers/media/platform/ti/cal/cal-video.c
+++ b/drivers/media/platform/ti/cal/cal-video.c
@@ -814,7 +814,6 @@ static const struct v4l2_file_operations cal_fops = {
static int cal_ctx_v4l2_init_formats(struct cal_ctx *ctx)
{
- struct v4l2_subdev_mbus_code_enum mbus_code;
struct v4l2_mbus_framefmt mbus_fmt;
const struct cal_format_info *fmtinfo;
unsigned int i, j, k;
@@ -829,10 +828,11 @@ static int cal_ctx_v4l2_init_formats(struct cal_ctx *ctx)
ctx->num_active_fmt = 0;
for (j = 0, i = 0; ; ++j) {
+ struct v4l2_subdev_mbus_code_enum mbus_code = {
+ .index = j,
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
- memset(&mbus_code, 0, sizeof(mbus_code));
- mbus_code.index = j;
- mbus_code.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(ctx->phy->source, pad, enum_mbus_code,
NULL, &mbus_code);
if (ret == -EINVAL)
diff --git a/drivers/media/usb/dvb-usb/cxusb-analog.c b/drivers/media/usb/dvb-usb/cxusb-analog.c
index e93183ddd797..deba5224cb8d 100644
--- a/drivers/media/usb/dvb-usb/cxusb-analog.c
+++ b/drivers/media/usb/dvb-usb/cxusb-analog.c
@@ -1014,7 +1014,10 @@ static int cxusb_medion_try_s_fmt_vid_cap(struct file *file,
{
struct dvb_usb_device *dvbdev = video_drvdata(file);
struct cxusb_medion_dev *cxdev = dvbdev->priv;
- struct v4l2_subdev_format subfmt;
+ struct v4l2_subdev_format subfmt = {
+ .which = isset ? V4L2_SUBDEV_FORMAT_ACTIVE :
+ V4L2_SUBDEV_FORMAT_TRY,
+ };
u32 field;
int ret;
@@ -1024,9 +1027,6 @@ static int cxusb_medion_try_s_fmt_vid_cap(struct file *file,
field = vb2_start_streaming_called(&cxdev->videoqueue) ?
cxdev->field_order : cxusb_medion_field_order(cxdev);
- memset(&subfmt, 0, sizeof(subfmt));
- subfmt.which = isset ? V4L2_SUBDEV_FORMAT_ACTIVE :
- V4L2_SUBDEV_FORMAT_TRY;
subfmt.format.width = f->fmt.pix.width & ~1;
subfmt.format.height = f->fmt.pix.height & ~1;
subfmt.format.code = MEDIA_BUS_FMT_FIXED;
@@ -1464,7 +1464,9 @@ int cxusb_medion_analog_init(struct dvb_usb_device *dvbdev)
.buf = tuner_analog_msg_data,
.len =
sizeof(tuner_analog_msg_data) };
- struct v4l2_subdev_format subfmt;
+ struct v4l2_subdev_format subfmt = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
int ret;
/* switch tuner to analog mode so IF demod will become accessible */
@@ -1507,8 +1509,6 @@ int cxusb_medion_analog_init(struct dvb_usb_device *dvbdev)
v4l2_subdev_call(cxdev->tuner, video, s_std, cxdev->norm);
v4l2_subdev_call(cxdev->cx25840, video, s_std, cxdev->norm);
- memset(&subfmt, 0, sizeof(subfmt));
- subfmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
subfmt.format.width = cxdev->width;
subfmt.format.height = cxdev->height;
subfmt.format.code = MEDIA_BUS_FMT_FIXED;
diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
index 39666db59c4e..4364df27c6d2 100644
--- a/drivers/staging/media/imx/imx-media-capture.c
+++ b/drivers/staging/media/imx/imx-media-capture.c
@@ -504,14 +504,14 @@ static int capture_legacy_g_parm(struct file *file, void *fh,
struct v4l2_streamparm *a)
{
struct capture_priv *priv = video_drvdata(file);
- struct v4l2_subdev_frame_interval fi;
+ struct v4l2_subdev_frame_interval fi = {
+ .pad = priv->src_sd_pad,
+ };
int ret;
if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
return -EINVAL;
- memset(&fi, 0, sizeof(fi));
- fi.pad = priv->src_sd_pad;
ret = v4l2_subdev_call(priv->src_sd, video, g_frame_interval, &fi);
if (ret < 0)
return ret;
@@ -526,14 +526,14 @@ static int capture_legacy_s_parm(struct file *file, void *fh,
struct v4l2_streamparm *a)
{
struct capture_priv *priv = video_drvdata(file);
- struct v4l2_subdev_frame_interval fi;
+ struct v4l2_subdev_frame_interval fi = {
+ .pad = priv->src_sd_pad,
+ };
int ret;
if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
return -EINVAL;
- memset(&fi, 0, sizeof(fi));
- fi.pad = priv->src_sd_pad;
fi.interval = a->parm.capture.timeperframe;
ret = v4l2_subdev_call(priv->src_sd, video, s_frame_interval, &fi);
if (ret < 0)
diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index 411e907b68eb..b545750ca526 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -432,15 +432,15 @@ int imx_media_init_cfg(struct v4l2_subdev *sd,
struct v4l2_subdev_state *sd_state)
{
struct v4l2_mbus_framefmt *mf_try;
- struct v4l2_subdev_format format;
unsigned int pad;
int ret;
for (pad = 0; pad < sd->entity.num_pads; pad++) {
- memset(&format, 0, sizeof(format));
+ struct v4l2_subdev_format format = {
+ .pad = pad,
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
- format.pad = pad;
- format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &format);
if (ret)
continue;
diff --git a/drivers/staging/media/omap4iss/iss_video.c b/drivers/staging/media/omap4iss/iss_video.c
index 1406445a30c3..22fa4d6cae10 100644
--- a/drivers/staging/media/omap4iss/iss_video.c
+++ b/drivers/staging/media/omap4iss/iss_video.c
@@ -237,7 +237,9 @@ static int
__iss_video_get_format(struct iss_video *video,
struct v4l2_mbus_framefmt *format)
{
- struct v4l2_subdev_format fmt;
+ struct v4l2_subdev_format fmt = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
struct v4l2_subdev *subdev;
u32 pad;
int ret;
@@ -246,9 +248,7 @@ __iss_video_get_format(struct iss_video *video,
if (!subdev)
return -EINVAL;
- memset(&fmt, 0, sizeof(fmt));
fmt.pad = pad;
- fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
mutex_lock(&video->mutex);
ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/3] media: USe designated initializers for all subdev pad ops
2023-02-15 16:50 [PATCH 0/3] media: Zero-initialize structures passed to subdev pad ops Laurent Pinchart
2023-02-15 16:50 ` [PATCH 1/3] media: Zero-initialize all structures passed to subdev pad operations Laurent Pinchart
2023-02-15 16:50 ` [PATCH 2/3] media: Prefer designated initializers over memset for subdev pad ops Laurent Pinchart
@ 2023-02-15 16:50 ` Laurent Pinchart
2023-02-28 15:52 ` [PATCH 0/3] media: Zero-initialize structures passed to " Tomi Valkeinen
2023-02-28 16:41 ` Kieran Bingham
4 siblings, 0 replies; 16+ messages in thread
From: Laurent Pinchart @ 2023-02-15 16:50 UTC (permalink / raw)
To: linux-media
Cc: Hans Verkuil, Sakari Ailus, Tomi Valkeinen, Yong Zhi, Bingbu Cao,
Dan Scally, Tianshu Qiu, Eugen Hristev, Robert Foss, Todor Tomov,
Kieran Bingham, Sylwester Nawrocki, Prabhakar Lad, Benoit Parrot,
Shuah Khan, Michael Krufky, Steve Longerbeam, Philipp Zabel,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni, kernel,
linux-imx
Structures passed to subdev pad operations are all zero-initialized when
declaring variables. In most cases, this is done with designated
initializers to initialize some of the fields to specific values, but in
a minority of cases the structures are zero-initialized by assigning
them to '{ 0 }' or '{ }'.
Improve coding style consistency by using designated initializers where
possible, always initializing the 'which' field.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/pci/cobalt/cobalt-v4l2.c | 5 +++--
drivers/media/platform/microchip/microchip-isc-base.c | 5 +++--
drivers/media/platform/renesas/vsp1/vsp1_drm.c | 5 +++--
drivers/staging/media/deprecated/atmel/atmel-isc-base.c | 5 +++--
4 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/media/pci/cobalt/cobalt-v4l2.c b/drivers/media/pci/cobalt/cobalt-v4l2.c
index 284c0909a282..4bfbcca14f60 100644
--- a/drivers/media/pci/cobalt/cobalt-v4l2.c
+++ b/drivers/media/pci/cobalt/cobalt-v4l2.c
@@ -910,7 +910,9 @@ static int cobalt_s_fmt_vid_out(struct file *file, void *priv_fh,
{
struct cobalt_stream *s = video_drvdata(file);
struct v4l2_pix_format *pix = &f->fmt.pix;
- struct v4l2_subdev_format sd_fmt = { 0 };
+ struct v4l2_subdev_format sd_fmt = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
u32 code;
if (cobalt_try_fmt_vid_out(file, priv_fh, f))
@@ -941,7 +943,6 @@ static int cobalt_s_fmt_vid_out(struct file *file, void *priv_fh,
s->xfer_func = pix->xfer_func;
s->ycbcr_enc = pix->ycbcr_enc;
s->quantization = pix->quantization;
- sd_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
v4l2_fill_mbus_format(&sd_fmt.format, pix, code);
v4l2_subdev_call(s->sd, pad, set_fmt, NULL, &sd_fmt);
return 0;
diff --git a/drivers/media/platform/microchip/microchip-isc-base.c b/drivers/media/platform/microchip/microchip-isc-base.c
index 71758ee8474b..4e657fad33d0 100644
--- a/drivers/media/platform/microchip/microchip-isc-base.c
+++ b/drivers/media/platform/microchip/microchip-isc-base.c
@@ -858,8 +858,10 @@ static int isc_try_configure_pipeline(struct isc_device *isc)
static void isc_try_fse(struct isc_device *isc,
struct v4l2_subdev_state *sd_state)
{
+ struct v4l2_subdev_frame_size_enum fse = {
+ .which = V4L2_SUBDEV_FORMAT_TRY,
+ };
int ret;
- struct v4l2_subdev_frame_size_enum fse = {};
/*
* If we do not know yet which format the subdev is using, we cannot
@@ -869,7 +871,6 @@ static void isc_try_fse(struct isc_device *isc,
return;
fse.code = isc->try_config.sd_format->mbus_code;
- fse.which = V4L2_SUBDEV_FORMAT_TRY;
ret = v4l2_subdev_call(isc->current_subdev->sd, pad, enum_frame_size,
sd_state, &fse);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drm.c b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
index 7fe375b6322c..24979d604baa 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
@@ -481,10 +481,11 @@ static int vsp1_du_pipeline_setup_output(struct vsp1_device *vsp1,
struct vsp1_pipeline *pipe)
{
struct vsp1_drm_pipeline *drm_pipe = to_vsp1_drm_pipeline(pipe);
- struct v4l2_subdev_format format = { 0, };
+ struct v4l2_subdev_format format = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
int ret;
- format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
format.pad = RWPF_PAD_SINK;
format.format.width = drm_pipe->width;
format.format.height = drm_pipe->height;
diff --git a/drivers/staging/media/deprecated/atmel/atmel-isc-base.c b/drivers/staging/media/deprecated/atmel/atmel-isc-base.c
index 0d48ae1bd71a..61c5afa58142 100644
--- a/drivers/staging/media/deprecated/atmel/atmel-isc-base.c
+++ b/drivers/staging/media/deprecated/atmel/atmel-isc-base.c
@@ -824,8 +824,10 @@ static int isc_try_configure_pipeline(struct isc_device *isc)
static void isc_try_fse(struct isc_device *isc,
struct v4l2_subdev_state *sd_state)
{
+ struct v4l2_subdev_frame_size_enum fse = {
+ .which = V4L2_SUBDEV_FORMAT_TRY,
+ };
int ret;
- struct v4l2_subdev_frame_size_enum fse = {};
/*
* If we do not know yet which format the subdev is using, we cannot
@@ -835,7 +837,6 @@ static void isc_try_fse(struct isc_device *isc,
return;
fse.code = isc->try_config.sd_format->mbus_code;
- fse.which = V4L2_SUBDEV_FORMAT_TRY;
ret = v4l2_subdev_call(isc->current_subdev->sd, pad, enum_frame_size,
sd_state, &fse);
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] media: Zero-initialize all structures passed to subdev pad operations
2023-02-15 16:50 ` [PATCH 1/3] media: Zero-initialize all structures passed to subdev pad operations Laurent Pinchart
@ 2023-02-27 22:30 ` Shuah Khan
2023-02-28 9:05 ` Lad, Prabhakar
` (2 subsequent siblings)
3 siblings, 0 replies; 16+ messages in thread
From: Shuah Khan @ 2023-02-27 22:30 UTC (permalink / raw)
To: Laurent Pinchart, linux-media
Cc: Hans Verkuil, Sakari Ailus, Tomi Valkeinen, Yong Zhi, Bingbu Cao,
Dan Scally, Tianshu Qiu, Eugen Hristev, Robert Foss, Todor Tomov,
Kieran Bingham, Sylwester Nawrocki, Prabhakar Lad, Benoit Parrot,
Michael Krufky, Steve Longerbeam, Philipp Zabel, Thierry Reding,
Jonathan Hunter, Sowjanya Komatineni, kernel, linux-imx,
Shuah Khan
On 2/15/23 09:50, Laurent Pinchart wrote:
> Several drivers call subdev pad operations, passing structures that are
> not fully zeroed. While the drivers initialize the fields they care
> about explicitly, this results in reserved fields having uninitialized
> values. Future kernel API changes that make use of those fields thus
> risk breaking proper driver operation in ways that could be hard to
> detect.
>
> To avoid this, make the code more robust by zero-initializing all the
> structures passed to subdev pad operation. Maintain a consistent coding
> style by preferring designated initializers (which zero-initialize all
> the fields that are not specified) over memset() where possible, and
> make variable declarations local to inner scopes where applicable. One
> notable exception to this rule is in the ipu3 driver, where a memset()
> is needed as the structure is not a local variable but a function
> parameter provided by the caller.
>
> Not all fields of those structures can be initialized when declaring the
> variables, as the values for those fields are computed later in the
> code. Initialize the 'which' field in all cases, and other fields when
> the variable declaration is so close to the v4l2_subdev_call() call that
> it keeps all the context easily visible when reading the code, to avoid
> hindering readability.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/media/pci/cobalt/cobalt-v4l2.c | 16 ++++++----
> drivers/media/pci/intel/ipu3/ipu3-cio2-main.c | 1 +
> .../media/platform/qcom/camss/camss-video.c | 5 ++--
> .../media/platform/renesas/vsp1/vsp1_video.c | 5 ++--
> .../samsung/exynos4-is/fimc-capture.c | 11 ++++---
> .../samsung/exynos4-is/fimc-isp-video.c | 10 +++++--
> .../platform/samsung/exynos4-is/fimc-lite.c | 9 ++++--
> .../samsung/s3c-camif/camif-capture.c | 5 ++--
> .../platform/samsung/s3c-camif/camif-core.c | 5 ++--
> .../media/platform/ti/am437x/am437x-vpfe.c | 20 +++++++------
> drivers/media/platform/ti/cal/cal-video.c | 29 ++++++++++---------
> drivers/media/platform/ti/omap3isp/ispccdc.c | 5 ++--
> drivers/media/platform/ti/omap3isp/ispvideo.c | 20 ++++++++-----
> drivers/media/platform/xilinx/xilinx-dma.c | 5 ++--
> drivers/media/test-drivers/vimc/vimc-common.c | 8 ++---
For vimc
Acked-by: Shuah Khan <skhan@linuxfoundation.org>
thanks,
-- Shuah
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] media: Zero-initialize all structures passed to subdev pad operations
2023-02-15 16:50 ` [PATCH 1/3] media: Zero-initialize all structures passed to subdev pad operations Laurent Pinchart
2023-02-27 22:30 ` Shuah Khan
@ 2023-02-28 9:05 ` Lad, Prabhakar
2023-02-28 10:05 ` Sakari Ailus
2023-03-06 13:00 ` Philipp Zabel
3 siblings, 0 replies; 16+ messages in thread
From: Lad, Prabhakar @ 2023-02-28 9:05 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Hans Verkuil, Sakari Ailus, Tomi Valkeinen, Yong Zhi,
Bingbu Cao, Dan Scally, Tianshu Qiu, Eugen Hristev, Robert Foss,
Todor Tomov, Kieran Bingham, Sylwester Nawrocki, Benoit Parrot,
Shuah Khan, Michael Krufky, Steve Longerbeam, Philipp Zabel,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni, kernel,
linux-imx
Hi Laurent,
Thank you for the patch.
On Wed, Feb 15, 2023 at 4:50 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Several drivers call subdev pad operations, passing structures that are
> not fully zeroed. While the drivers initialize the fields they care
> about explicitly, this results in reserved fields having uninitialized
> values. Future kernel API changes that make use of those fields thus
> risk breaking proper driver operation in ways that could be hard to
> detect.
>
> To avoid this, make the code more robust by zero-initializing all the
> structures passed to subdev pad operation. Maintain a consistent coding
> style by preferring designated initializers (which zero-initialize all
> the fields that are not specified) over memset() where possible, and
> make variable declarations local to inner scopes where applicable. One
> notable exception to this rule is in the ipu3 driver, where a memset()
> is needed as the structure is not a local variable but a function
> parameter provided by the caller.
>
> Not all fields of those structures can be initialized when declaring the
> variables, as the values for those fields are computed later in the
> code. Initialize the 'which' field in all cases, and other fields when
> the variable declaration is so close to the v4l2_subdev_call() call that
> it keeps all the context easily visible when reading the code, to avoid
> hindering readability.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
<snip>
> .../media/platform/ti/am437x/am437x-vpfe.c | 20 +++++++------
For above,
Reviewed-by: Lad Prabhakar <prabhakar.csengg@gmail.com>
Cheers,
Prabhakar
> drivers/media/platform/ti/cal/cal-video.c | 29 ++++++++++---------
> drivers/media/platform/ti/omap3isp/ispccdc.c | 5 ++--
> drivers/media/platform/ti/omap3isp/ispvideo.c | 20 ++++++++-----
> drivers/media/platform/xilinx/xilinx-dma.c | 5 ++--
> drivers/media/test-drivers/vimc/vimc-common.c | 8 ++---
> drivers/staging/media/imx/imx-media-capture.c | 28 ++++++++++--------
> drivers/staging/media/omap4iss/iss_video.c | 10 ++++---
> drivers/staging/media/tegra-video/vi.c | 10 ++++---
> 18 files changed, 120 insertions(+), 82 deletions(-)
>
> diff --git a/drivers/media/pci/cobalt/cobalt-v4l2.c b/drivers/media/pci/cobalt/cobalt-v4l2.c
> index 0ff37496c9ab..284c0909a282 100644
> --- a/drivers/media/pci/cobalt/cobalt-v4l2.c
> +++ b/drivers/media/pci/cobalt/cobalt-v4l2.c
> @@ -708,7 +708,6 @@ static int cobalt_g_fmt_vid_cap(struct file *file, void *priv_fh,
> {
> struct cobalt_stream *s = video_drvdata(file);
> struct v4l2_pix_format *pix = &f->fmt.pix;
> - struct v4l2_subdev_format sd_fmt;
>
> pix->width = s->width;
> pix->height = s->height;
> @@ -718,8 +717,11 @@ static int cobalt_g_fmt_vid_cap(struct file *file, void *priv_fh,
> if (s->input == 1) {
> pix->colorspace = V4L2_COLORSPACE_SRGB;
> } else {
> - sd_fmt.pad = s->pad_source;
> - sd_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> + struct v4l2_subdev_format sd_fmt = {
> + .pad = s->pad_source,
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> +
> v4l2_subdev_call(s->sd, pad, get_fmt, NULL, &sd_fmt);
> v4l2_fill_pix_format(pix, &sd_fmt.format);
> }
> @@ -735,7 +737,6 @@ static int cobalt_try_fmt_vid_cap(struct file *file, void *priv_fh,
> {
> struct cobalt_stream *s = video_drvdata(file);
> struct v4l2_pix_format *pix = &f->fmt.pix;
> - struct v4l2_subdev_format sd_fmt;
>
> /* Check for min (QCIF) and max (Full HD) size */
> if ((pix->width < 176) || (pix->height < 144)) {
> @@ -760,8 +761,11 @@ static int cobalt_try_fmt_vid_cap(struct file *file, void *priv_fh,
> pix->height = 1080;
> pix->colorspace = V4L2_COLORSPACE_SRGB;
> } else {
> - sd_fmt.pad = s->pad_source;
> - sd_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> + struct v4l2_subdev_format sd_fmt = {
> + .pad = s->pad_source,
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> +
> v4l2_subdev_call(s->sd, pad, get_fmt, NULL, &sd_fmt);
> v4l2_fill_pix_format(pix, &sd_fmt.format);
> }
> diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
> index 3b76a9d0383a..3c84cb121632 100644
> --- a/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
> +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
> @@ -1305,6 +1305,7 @@ static int cio2_subdev_link_validate_get_format(struct media_pad *pad,
> struct v4l2_subdev *sd =
> media_entity_to_v4l2_subdev(pad->entity);
>
> + memset(fmt, 0, sizeof(*fmt));
> fmt->which = V4L2_SUBDEV_FORMAT_ACTIVE;
> fmt->pad = pad->index;
> return v4l2_subdev_call(sd, pad, get_fmt, NULL, fmt);
> diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
> index 41deda232e4a..65c87af24463 100644
> --- a/drivers/media/platform/qcom/camss/camss-video.c
> +++ b/drivers/media/platform/qcom/camss/camss-video.c
> @@ -342,7 +342,9 @@ static struct v4l2_subdev *video_remote_subdev(struct camss_video *video,
> static int video_get_subdev_format(struct camss_video *video,
> struct v4l2_format *format)
> {
> - struct v4l2_subdev_format fmt;
> + struct v4l2_subdev_format fmt = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> struct v4l2_subdev *subdev;
> u32 pad;
> int ret;
> @@ -352,7 +354,6 @@ static int video_get_subdev_format(struct camss_video *video,
> return -EPIPE;
>
> fmt.pad = pad;
> - fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
>
> ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);
> if (ret)
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c b/drivers/media/platform/renesas/vsp1/vsp1_video.c
> index 9d24647c8f32..ee0b61a52327 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_video.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c
> @@ -62,7 +62,9 @@ vsp1_video_remote_subdev(struct media_pad *local, u32 *pad)
>
> static int vsp1_video_verify_format(struct vsp1_video *video)
> {
> - struct v4l2_subdev_format fmt;
> + struct v4l2_subdev_format fmt = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> struct v4l2_subdev *subdev;
> int ret;
>
> @@ -70,7 +72,6 @@ static int vsp1_video_verify_format(struct vsp1_video *video)
> if (subdev == NULL)
> return -EINVAL;
>
> - fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);
> if (ret < 0)
> return ret == -ENOIOCTLCMD ? -EINVAL : ret;
> diff --git a/drivers/media/platform/samsung/exynos4-is/fimc-capture.c b/drivers/media/platform/samsung/exynos4-is/fimc-capture.c
> index e3b95a2b7e04..4800751a401c 100644
> --- a/drivers/media/platform/samsung/exynos4-is/fimc-capture.c
> +++ b/drivers/media/platform/samsung/exynos4-is/fimc-capture.c
> @@ -854,7 +854,7 @@ static int fimc_get_sensor_frame_desc(struct v4l2_subdev *sensor,
> struct v4l2_plane_pix_format *plane_fmt,
> unsigned int num_planes, bool try)
> {
> - struct v4l2_mbus_frame_desc fd;
> + struct v4l2_mbus_frame_desc fd = { };
> int i, ret;
> int pad;
>
> @@ -1095,7 +1095,12 @@ static int fimc_cap_g_input(struct file *file, void *priv, unsigned int *i)
> */
> static int fimc_pipeline_validate(struct fimc_dev *fimc)
> {
> - struct v4l2_subdev_format sink_fmt, src_fmt;
> + struct v4l2_subdev_format sink_fmt = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> + struct v4l2_subdev_format src_fmt = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> struct fimc_vid_cap *vc = &fimc->vid_cap;
> struct v4l2_subdev *sd = &vc->subdev;
> struct fimc_pipeline *p = to_fimc_pipeline(vc->ve.pipe);
> @@ -1132,7 +1137,6 @@ static int fimc_pipeline_validate(struct fimc_dev *fimc)
> sink_fmt.format.code = ff->fmt ? ff->fmt->mbus_code : 0;
> } else {
> sink_fmt.pad = sink_pad->index;
> - sink_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &sink_fmt);
> if (ret < 0 && ret != -ENOIOCTLCMD)
> return -EPIPE;
> @@ -1141,7 +1145,6 @@ static int fimc_pipeline_validate(struct fimc_dev *fimc)
> /* Retrieve format at the source pad */
> sd = media_entity_to_v4l2_subdev(src_pad->entity);
> src_fmt.pad = src_pad->index;
> - src_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &src_fmt);
> if (ret < 0 && ret != -ENOIOCTLCMD)
> return -EPIPE;
> diff --git a/drivers/media/platform/samsung/exynos4-is/fimc-isp-video.c b/drivers/media/platform/samsung/exynos4-is/fimc-isp-video.c
> index f6a302fa8d37..8fa26969c411 100644
> --- a/drivers/media/platform/samsung/exynos4-is/fimc-isp-video.c
> +++ b/drivers/media/platform/samsung/exynos4-is/fimc-isp-video.c
> @@ -449,17 +449,22 @@ static int isp_video_s_fmt_mplane(struct file *file, void *priv,
> static int isp_video_pipeline_validate(struct fimc_isp *isp)
> {
> struct v4l2_subdev *sd = &isp->subdev;
> - struct v4l2_subdev_format sink_fmt, src_fmt;
> struct media_pad *pad;
> int ret;
>
> while (1) {
> + struct v4l2_subdev_format sink_fmt = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> + struct v4l2_subdev_format src_fmt = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> +
> /* Retrieve format at the sink pad */
> pad = &sd->entity.pads[0];
> if (!(pad->flags & MEDIA_PAD_FL_SINK))
> break;
> sink_fmt.pad = pad->index;
> - sink_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &sink_fmt);
> if (ret < 0 && ret != -ENOIOCTLCMD)
> return -EPIPE;
> @@ -471,7 +476,6 @@ static int isp_video_pipeline_validate(struct fimc_isp *isp)
>
> sd = media_entity_to_v4l2_subdev(pad->entity);
> src_fmt.pad = pad->index;
> - src_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &src_fmt);
> if (ret < 0 && ret != -ENOIOCTLCMD)
> return -EPIPE;
> diff --git a/drivers/media/platform/samsung/exynos4-is/fimc-lite.c b/drivers/media/platform/samsung/exynos4-is/fimc-lite.c
> index e185a40305a8..162e44efcb3e 100644
> --- a/drivers/media/platform/samsung/exynos4-is/fimc-lite.c
> +++ b/drivers/media/platform/samsung/exynos4-is/fimc-lite.c
> @@ -765,7 +765,12 @@ static int fimc_lite_s_fmt_mplane(struct file *file, void *priv,
> static int fimc_pipeline_validate(struct fimc_lite *fimc)
> {
> struct v4l2_subdev *sd = &fimc->subdev;
> - struct v4l2_subdev_format sink_fmt, src_fmt;
> + struct v4l2_subdev_format sink_fmt = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> + struct v4l2_subdev_format src_fmt = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> struct media_pad *pad;
> int ret;
>
> @@ -782,7 +787,6 @@ static int fimc_pipeline_validate(struct fimc_lite *fimc)
> sink_fmt.format.code = fimc->inp_frame.fmt->mbus_code;
> } else {
> sink_fmt.pad = pad->index;
> - sink_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> ret = v4l2_subdev_call(sd, pad, get_fmt, NULL,
> &sink_fmt);
> if (ret < 0 && ret != -ENOIOCTLCMD)
> @@ -795,7 +799,6 @@ static int fimc_pipeline_validate(struct fimc_lite *fimc)
>
> sd = media_entity_to_v4l2_subdev(pad->entity);
> src_fmt.pad = pad->index;
> - src_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &src_fmt);
> if (ret < 0 && ret != -ENOIOCTLCMD)
> return -EPIPE;
> diff --git a/drivers/media/platform/samsung/s3c-camif/camif-capture.c b/drivers/media/platform/samsung/s3c-camif/camif-capture.c
> index db106ebdf870..76634d242b10 100644
> --- a/drivers/media/platform/samsung/s3c-camif/camif-capture.c
> +++ b/drivers/media/platform/samsung/s3c-camif/camif-capture.c
> @@ -806,7 +806,9 @@ static int s3c_camif_vidioc_s_fmt(struct file *file, void *priv,
> /* Only check pixel formats at the sensor and the camif subdev pads */
> static int camif_pipeline_validate(struct camif_dev *camif)
> {
> - struct v4l2_subdev_format src_fmt;
> + struct v4l2_subdev_format src_fmt = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> struct media_pad *pad;
> int ret;
>
> @@ -816,7 +818,6 @@ static int camif_pipeline_validate(struct camif_dev *camif)
> return -EPIPE;
>
> src_fmt.pad = pad->index;
> - src_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> ret = v4l2_subdev_call(camif->sensor.sd, pad, get_fmt, NULL, &src_fmt);
> if (ret < 0 && ret != -ENOIOCTLCMD)
> return -EPIPE;
> diff --git a/drivers/media/platform/samsung/s3c-camif/camif-core.c b/drivers/media/platform/samsung/s3c-camif/camif-core.c
> index 6e8ef86566b7..9827510b7b50 100644
> --- a/drivers/media/platform/samsung/s3c-camif/camif-core.c
> +++ b/drivers/media/platform/samsung/s3c-camif/camif-core.c
> @@ -190,7 +190,9 @@ static int camif_register_sensor(struct camif_dev *camif)
> struct s3c_camif_sensor_info *sensor = &camif->pdata.sensor;
> struct v4l2_device *v4l2_dev = &camif->v4l2_dev;
> struct i2c_adapter *adapter;
> - struct v4l2_subdev_format format;
> + struct v4l2_subdev_format format = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> struct v4l2_subdev *sd;
> int ret;
>
> @@ -220,7 +222,6 @@ static int camif_register_sensor(struct camif_dev *camif)
>
> /* Get initial pixel format and set it at the camif sink pad */
> format.pad = 0;
> - format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &format);
>
> if (ret < 0)
> diff --git a/drivers/media/platform/ti/am437x/am437x-vpfe.c b/drivers/media/platform/ti/am437x/am437x-vpfe.c
> index 2dfae9bc0bba..f23085b5954b 100644
> --- a/drivers/media/platform/ti/am437x/am437x-vpfe.c
> +++ b/drivers/media/platform/ti/am437x/am437x-vpfe.c
> @@ -1285,13 +1285,13 @@ static int __subdev_get_format(struct vpfe_device *vpfe,
> struct v4l2_mbus_framefmt *fmt)
> {
> struct v4l2_subdev *sd = vpfe->current_subdev->sd;
> - struct v4l2_subdev_format sd_fmt;
> + struct v4l2_subdev_format sd_fmt = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + .pad = 0,
> + };
> struct v4l2_mbus_framefmt *mbus_fmt = &sd_fmt.format;
> int ret;
>
> - sd_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> - sd_fmt.pad = 0;
> -
> ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &sd_fmt);
> if (ret)
> return ret;
> @@ -1309,12 +1309,13 @@ static int __subdev_set_format(struct vpfe_device *vpfe,
> struct v4l2_mbus_framefmt *fmt)
> {
> struct v4l2_subdev *sd = vpfe->current_subdev->sd;
> - struct v4l2_subdev_format sd_fmt;
> + struct v4l2_subdev_format sd_fmt = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + .pad = 0,
> + };
> struct v4l2_mbus_framefmt *mbus_fmt = &sd_fmt.format;
> int ret;
>
> - sd_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> - sd_fmt.pad = 0;
> *mbus_fmt = *fmt;
>
> ret = v4l2_subdev_call(sd, pad, set_fmt, NULL, &sd_fmt);
> @@ -1393,7 +1394,9 @@ static int vpfe_try_fmt(struct file *file, void *priv,
> struct vpfe_device *vpfe = video_drvdata(file);
> struct v4l2_subdev *sd = vpfe->current_subdev->sd;
> const struct vpfe_fmt *fmt;
> - struct v4l2_subdev_frame_size_enum fse;
> + struct v4l2_subdev_frame_size_enum fse = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> int ret, found;
>
> fmt = find_format_by_pix(vpfe, f->fmt.pix.pixelformat);
> @@ -1412,7 +1415,6 @@ static int vpfe_try_fmt(struct file *file, void *priv,
> found = false;
> fse.pad = 0;
> fse.code = fmt->code;
> - fse.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> for (fse.index = 0; ; fse.index++) {
> ret = v4l2_subdev_call(sd, pad, enum_frame_size,
> NULL, &fse);
> diff --git a/drivers/media/platform/ti/cal/cal-video.c b/drivers/media/platform/ti/cal/cal-video.c
> index 4eade409d5d3..51ed82172d71 100644
> --- a/drivers/media/platform/ti/cal/cal-video.c
> +++ b/drivers/media/platform/ti/cal/cal-video.c
> @@ -117,13 +117,13 @@ static int cal_legacy_enum_fmt_vid_cap(struct file *file, void *priv,
> static int __subdev_get_format(struct cal_ctx *ctx,
> struct v4l2_mbus_framefmt *fmt)
> {
> - struct v4l2_subdev_format sd_fmt;
> + struct v4l2_subdev_format sd_fmt = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + .pad = 0,
> + };
> struct v4l2_mbus_framefmt *mbus_fmt = &sd_fmt.format;
> int ret;
>
> - sd_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> - sd_fmt.pad = 0;
> -
> ret = v4l2_subdev_call(ctx->phy->source, pad, get_fmt, NULL, &sd_fmt);
> if (ret)
> return ret;
> @@ -139,12 +139,13 @@ static int __subdev_get_format(struct cal_ctx *ctx,
> static int __subdev_set_format(struct cal_ctx *ctx,
> struct v4l2_mbus_framefmt *fmt)
> {
> - struct v4l2_subdev_format sd_fmt;
> + struct v4l2_subdev_format sd_fmt = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + .pad = 0,
> + };
> struct v4l2_mbus_framefmt *mbus_fmt = &sd_fmt.format;
> int ret;
>
> - sd_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> - sd_fmt.pad = 0;
> *mbus_fmt = *fmt;
>
> ret = v4l2_subdev_call(ctx->phy->source, pad, set_fmt, NULL, &sd_fmt);
> @@ -190,7 +191,9 @@ static int cal_legacy_try_fmt_vid_cap(struct file *file, void *priv,
> {
> struct cal_ctx *ctx = video_drvdata(file);
> const struct cal_format_info *fmtinfo;
> - struct v4l2_subdev_frame_size_enum fse;
> + struct v4l2_subdev_frame_size_enum fse = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> int found;
>
> fmtinfo = find_format_by_pix(ctx, f->fmt.pix.pixelformat);
> @@ -209,7 +212,6 @@ static int cal_legacy_try_fmt_vid_cap(struct file *file, void *priv,
> found = false;
> fse.pad = 0;
> fse.code = fmtinfo->code;
> - fse.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> for (fse.index = 0; ; fse.index++) {
> int ret;
>
> @@ -302,7 +304,11 @@ static int cal_legacy_enum_framesizes(struct file *file, void *fh,
> {
> struct cal_ctx *ctx = video_drvdata(file);
> const struct cal_format_info *fmtinfo;
> - struct v4l2_subdev_frame_size_enum fse;
> + struct v4l2_subdev_frame_size_enum fse = {
> + .index = fsize->index,
> + .pad = 0,
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> int ret;
>
> /* check for valid format */
> @@ -313,10 +319,7 @@ static int cal_legacy_enum_framesizes(struct file *file, void *fh,
> return -EINVAL;
> }
>
> - fse.index = fsize->index;
> - fse.pad = 0;
> fse.code = fmtinfo->code;
> - fse.which = V4L2_SUBDEV_FORMAT_ACTIVE;
>
> ret = v4l2_subdev_call(ctx->phy->source, pad, enum_frame_size, NULL,
> &fse);
> diff --git a/drivers/media/platform/ti/omap3isp/ispccdc.c b/drivers/media/platform/ti/omap3isp/ispccdc.c
> index 11afb8aec292..fdcdffe5fecb 100644
> --- a/drivers/media/platform/ti/omap3isp/ispccdc.c
> +++ b/drivers/media/platform/ti/omap3isp/ispccdc.c
> @@ -1118,7 +1118,9 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
> struct v4l2_mbus_framefmt *format;
> const struct v4l2_rect *crop;
> const struct isp_format_info *fmt_info;
> - struct v4l2_subdev_format fmt_src;
> + struct v4l2_subdev_format fmt_src = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> unsigned int depth_out;
> unsigned int depth_in = 0;
> struct media_pad *pad;
> @@ -1150,7 +1152,6 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
> * input format is a non-BT.656 YUV variant.
> */
> fmt_src.pad = pad->index;
> - fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> if (!v4l2_subdev_call(sensor, pad, get_fmt, NULL, &fmt_src)) {
> fmt_info = omap3isp_video_format_info(fmt_src.format.code);
> depth_in = fmt_info->width;
> diff --git a/drivers/media/platform/ti/omap3isp/ispvideo.c b/drivers/media/platform/ti/omap3isp/ispvideo.c
> index ddc7d08d4f96..daca689dc082 100644
> --- a/drivers/media/platform/ti/omap3isp/ispvideo.c
> +++ b/drivers/media/platform/ti/omap3isp/ispvideo.c
> @@ -268,7 +268,9 @@ static int isp_video_get_graph_data(struct isp_video *video,
> static int
> __isp_video_get_format(struct isp_video *video, struct v4l2_format *format)
> {
> - struct v4l2_subdev_format fmt;
> + struct v4l2_subdev_format fmt = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> struct v4l2_subdev *subdev;
> u32 pad;
> int ret;
> @@ -278,7 +280,6 @@ __isp_video_get_format(struct isp_video *video, struct v4l2_format *format)
> return -EINVAL;
>
> fmt.pad = pad;
> - fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
>
> mutex_lock(&video->mutex);
> ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);
> @@ -731,7 +732,9 @@ static int
> isp_video_try_format(struct file *file, void *fh, struct v4l2_format *format)
> {
> struct isp_video *video = video_drvdata(file);
> - struct v4l2_subdev_format fmt;
> + struct v4l2_subdev_format fmt = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> struct v4l2_subdev *subdev;
> u32 pad;
> int ret;
> @@ -746,7 +749,6 @@ isp_video_try_format(struct file *file, void *fh, struct v4l2_format *format)
> isp_video_pix_to_mbus(&format->fmt.pix, &fmt.format);
>
> fmt.pad = pad;
> - fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);
> if (ret)
> return ret == -ENOIOCTLCMD ? -ENOTTY : ret;
> @@ -759,7 +761,9 @@ static int
> isp_video_get_selection(struct file *file, void *fh, struct v4l2_selection *sel)
> {
> struct isp_video *video = video_drvdata(file);
> - struct v4l2_subdev_format format;
> + struct v4l2_subdev_format format = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> struct v4l2_subdev *subdev;
> struct v4l2_subdev_selection sdsel = {
> .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> @@ -799,7 +803,6 @@ isp_video_get_selection(struct file *file, void *fh, struct v4l2_selection *sel)
> return ret;
>
> format.pad = pad;
> - format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &format);
> if (ret < 0)
> return ret == -ENOIOCTLCMD ? -ENOTTY : ret;
> @@ -957,7 +960,9 @@ static int isp_video_check_external_subdevs(struct isp_video *video,
> struct media_pad *source_pad;
> struct media_entity *source = NULL;
> struct media_entity *sink;
> - struct v4l2_subdev_format fmt;
> + struct v4l2_subdev_format fmt = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> struct v4l2_ext_controls ctrls;
> struct v4l2_ext_control ctrl;
> unsigned int i;
> @@ -993,7 +998,6 @@ static int isp_video_check_external_subdevs(struct isp_video *video,
> pipe->external = media_entity_to_v4l2_subdev(source);
>
> fmt.pad = source_pad->index;
> - fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> ret = v4l2_subdev_call(media_entity_to_v4l2_subdev(sink),
> pad, get_fmt, NULL, &fmt);
> if (unlikely(ret < 0)) {
> diff --git a/drivers/media/platform/xilinx/xilinx-dma.c b/drivers/media/platform/xilinx/xilinx-dma.c
> index fee02c8c85fd..80d6f5b072ea 100644
> --- a/drivers/media/platform/xilinx/xilinx-dma.c
> +++ b/drivers/media/platform/xilinx/xilinx-dma.c
> @@ -56,7 +56,9 @@ xvip_dma_remote_subdev(struct media_pad *local, u32 *pad)
>
> static int xvip_dma_verify_format(struct xvip_dma *dma)
> {
> - struct v4l2_subdev_format fmt;
> + struct v4l2_subdev_format fmt = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> struct v4l2_subdev *subdev;
> int ret;
>
> @@ -64,7 +66,6 @@ static int xvip_dma_verify_format(struct xvip_dma *dma)
> if (subdev == NULL)
> return -EPIPE;
>
> - fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);
> if (ret < 0)
> return ret == -ENOIOCTLCMD ? -EINVAL : ret;
> diff --git a/drivers/media/test-drivers/vimc/vimc-common.c b/drivers/media/test-drivers/vimc/vimc-common.c
> index 7b27153c0728..2e72974e35b4 100644
> --- a/drivers/media/test-drivers/vimc/vimc-common.c
> +++ b/drivers/media/test-drivers/vimc/vimc-common.c
> @@ -241,13 +241,13 @@ static int vimc_get_pix_format(struct media_pad *pad,
> if (is_media_entity_v4l2_subdev(pad->entity)) {
> struct v4l2_subdev *sd =
> media_entity_to_v4l2_subdev(pad->entity);
> - struct v4l2_subdev_format sd_fmt;
> + struct v4l2_subdev_format sd_fmt = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + .pad = pad->index,
> + };
> const struct vimc_pix_map *pix_map;
> int ret;
>
> - sd_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> - sd_fmt.pad = pad->index;
> -
> ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &sd_fmt);
> if (ret)
> return ret;
> diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
> index 93ba09236010..39666db59c4e 100644
> --- a/drivers/staging/media/imx/imx-media-capture.c
> +++ b/drivers/staging/media/imx/imx-media-capture.c
> @@ -353,12 +353,13 @@ static int capture_legacy_enum_fmt_vid_cap(struct file *file, void *fh,
> {
> struct capture_priv *priv = video_drvdata(file);
> const struct imx_media_pixfmt *cc_src;
> - struct v4l2_subdev_format fmt_src;
> + struct v4l2_subdev_format fmt_src = {
> + .pad = priv->src_sd_pad,
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> u32 fourcc;
> int ret;
>
> - fmt_src.pad = priv->src_sd_pad;
> - fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> ret = v4l2_subdev_call(priv->src_sd, pad, get_fmt, NULL, &fmt_src);
> if (ret) {
> dev_err(priv->dev, "failed to get src_sd format\n");
> @@ -426,11 +427,12 @@ static int capture_legacy_try_fmt_vid_cap(struct file *file, void *fh,
> struct v4l2_format *f)
> {
> struct capture_priv *priv = video_drvdata(file);
> - struct v4l2_subdev_format fmt_src;
> + struct v4l2_subdev_format fmt_src = {
> + .pad = priv->src_sd_pad,
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> int ret;
>
> - fmt_src.pad = priv->src_sd_pad;
> - fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> ret = v4l2_subdev_call(priv->src_sd, pad, get_fmt, NULL, &fmt_src);
> if (ret)
> return ret;
> @@ -445,7 +447,10 @@ static int capture_legacy_s_fmt_vid_cap(struct file *file, void *fh,
> struct v4l2_format *f)
> {
> struct capture_priv *priv = video_drvdata(file);
> - struct v4l2_subdev_format fmt_src;
> + struct v4l2_subdev_format fmt_src = {
> + .pad = priv->src_sd_pad,
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> const struct imx_media_pixfmt *cc;
> int ret;
>
> @@ -454,8 +459,6 @@ static int capture_legacy_s_fmt_vid_cap(struct file *file, void *fh,
> return -EBUSY;
> }
>
> - fmt_src.pad = priv->src_sd_pad;
> - fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> ret = v4l2_subdev_call(priv->src_sd, pad, get_fmt, NULL, &fmt_src);
> if (ret)
> return ret;
> @@ -670,13 +673,14 @@ static void capture_buf_queue(struct vb2_buffer *vb)
>
> static int capture_validate_fmt(struct capture_priv *priv)
> {
> - struct v4l2_subdev_format fmt_src;
> + struct v4l2_subdev_format fmt_src = {
> + .pad = priv->src_sd_pad,
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> const struct imx_media_pixfmt *cc;
> int ret;
>
> /* Retrieve the media bus format on the source subdev. */
> - fmt_src.pad = priv->src_sd_pad;
> - fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> ret = v4l2_subdev_call(priv->src_sd, pad, get_fmt, NULL, &fmt_src);
> if (ret)
> return ret;
> diff --git a/drivers/staging/media/omap4iss/iss_video.c b/drivers/staging/media/omap4iss/iss_video.c
> index 05548eab7daa..1406445a30c3 100644
> --- a/drivers/staging/media/omap4iss/iss_video.c
> +++ b/drivers/staging/media/omap4iss/iss_video.c
> @@ -610,7 +610,9 @@ static int
> iss_video_try_format(struct file *file, void *fh, struct v4l2_format *format)
> {
> struct iss_video *video = video_drvdata(file);
> - struct v4l2_subdev_format fmt;
> + struct v4l2_subdev_format fmt = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> struct v4l2_subdev *subdev;
> u32 pad;
> int ret;
> @@ -625,7 +627,6 @@ iss_video_try_format(struct file *file, void *fh, struct v4l2_format *format)
> iss_video_pix_to_mbus(&format->fmt.pix, &fmt.format);
>
> fmt.pad = pad;
> - fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);
> if (ret)
> return ret;
> @@ -638,7 +639,9 @@ static int
> iss_video_get_selection(struct file *file, void *fh, struct v4l2_selection *sel)
> {
> struct iss_video *video = video_drvdata(file);
> - struct v4l2_subdev_format format;
> + struct v4l2_subdev_format format = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> struct v4l2_subdev *subdev;
> struct v4l2_subdev_selection sdsel = {
> .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> @@ -679,7 +682,6 @@ iss_video_get_selection(struct file *file, void *fh, struct v4l2_selection *sel)
> return ret;
>
> format.pad = pad;
> - format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &format);
> if (ret < 0)
> return ret == -ENOIOCTLCMD ? -ENOTTY : ret;
> diff --git a/drivers/staging/media/tegra-video/vi.c b/drivers/staging/media/tegra-video/vi.c
> index 11dd142c98c5..8504adff9919 100644
> --- a/drivers/staging/media/tegra-video/vi.c
> +++ b/drivers/staging/media/tegra-video/vi.c
> @@ -493,7 +493,9 @@ static int __tegra_channel_try_format(struct tegra_vi_channel *chan,
> const struct tegra_video_format *fmtinfo;
> static struct lock_class_key key;
> struct v4l2_subdev *subdev;
> - struct v4l2_subdev_format fmt;
> + struct v4l2_subdev_format fmt = {
> + .which = V4L2_SUBDEV_FORMAT_TRY,
> + };
> struct v4l2_subdev_state *sd_state;
> struct v4l2_subdev_frame_size_enum fse = {
> .which = V4L2_SUBDEV_FORMAT_TRY,
> @@ -529,7 +531,6 @@ static int __tegra_channel_try_format(struct tegra_vi_channel *chan,
> }
>
> pix->field = V4L2_FIELD_NONE;
> - fmt.which = V4L2_SUBDEV_FORMAT_TRY;
> fmt.pad = 0;
> v4l2_fill_mbus_format(&fmt.format, pix, fmtinfo->code);
>
> @@ -590,7 +591,9 @@ static int tegra_channel_set_format(struct file *file, void *fh,
> {
> struct tegra_vi_channel *chan = video_drvdata(file);
> const struct tegra_video_format *fmtinfo;
> - struct v4l2_subdev_format fmt;
> + struct v4l2_subdev_format fmt = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> struct v4l2_subdev *subdev;
> struct v4l2_pix_format *pix = &format->fmt.pix;
> int ret;
> @@ -605,7 +608,6 @@ static int tegra_channel_set_format(struct file *file, void *fh,
>
> fmtinfo = tegra_get_format_by_fourcc(chan->vi, pix->pixelformat);
>
> - fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> fmt.pad = 0;
> v4l2_fill_mbus_format(&fmt.format, pix, fmtinfo->code);
> subdev = tegra_channel_get_remote_source_subdev(chan);
> --
> Regards,
>
> Laurent Pinchart
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] media: Prefer designated initializers over memset for subdev pad ops
2023-02-15 16:50 ` [PATCH 2/3] media: Prefer designated initializers over memset for subdev pad ops Laurent Pinchart
@ 2023-02-28 9:09 ` Lad, Prabhakar
2023-03-06 13:01 ` Philipp Zabel
1 sibling, 0 replies; 16+ messages in thread
From: Lad, Prabhakar @ 2023-02-28 9:09 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Hans Verkuil, Sakari Ailus, Tomi Valkeinen, Yong Zhi,
Bingbu Cao, Dan Scally, Tianshu Qiu, Eugen Hristev, Robert Foss,
Todor Tomov, Kieran Bingham, Sylwester Nawrocki, Benoit Parrot,
Shuah Khan, Michael Krufky, Steve Longerbeam, Philipp Zabel,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni, kernel,
linux-imx
Hi Laurent,
Thank you for the patch.
On Wed, Feb 15, 2023 at 4:50 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Structures passed to subdev pad operations are all zero-initialized, but
> not always with the same kind of code constructs. While most drivers
> used designated initializers, which zero all the fields that are not
> specified, when declaring variables, some use memset(). Those two
> methods lead to the same end result, and, depending on compiler
> optimizations, may even be completely equivalent, but they're not
> consistent.
>
> Improve coding style consistency by using designated initializers
> instead of calling memset(). Where applicable, also move the variables
> to inner scopes of for loops to ensure correct initialization in all
> iterations.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
<snip>
> drivers/media/platform/ti/am437x/am437x-vpfe.c | 15 ++++++++-------
For above,
Reviewed-by: Lad Prabhakar <prabhakar.csengg@gmail.com>
Cheers,
Prabhakar
> drivers/media/platform/ti/cal/cal-video.c | 8 ++++----
> drivers/media/usb/dvb-usb/cxusb-analog.c | 14 +++++++-------
> drivers/staging/media/imx/imx-media-capture.c | 12 ++++++------
> drivers/staging/media/imx/imx-media-utils.c | 8 ++++----
> drivers/staging/media/omap4iss/iss_video.c | 6 +++---
> 9 files changed, 50 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drm.c b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
> index c6f25200982c..7fe375b6322c 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
> @@ -66,7 +66,9 @@ static int vsp1_du_insert_uif(struct vsp1_device *vsp1,
> struct vsp1_entity *prev, unsigned int prev_pad,
> struct vsp1_entity *next, unsigned int next_pad)
> {
> - struct v4l2_subdev_format format;
> + struct v4l2_subdev_format format = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> int ret;
>
> if (!uif) {
> @@ -82,8 +84,6 @@ static int vsp1_du_insert_uif(struct vsp1_device *vsp1,
> prev->sink = uif;
> prev->sink_pad = UIF_PAD_SINK;
>
> - memset(&format, 0, sizeof(format));
> - format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> format.pad = prev_pad;
>
> ret = v4l2_subdev_call(&prev->subdev, pad, get_fmt, NULL, &format);
> @@ -118,8 +118,12 @@ static int vsp1_du_pipeline_setup_rpf(struct vsp1_device *vsp1,
> struct vsp1_entity *uif,
> unsigned int brx_input)
> {
> - struct v4l2_subdev_selection sel;
> - struct v4l2_subdev_format format;
> + struct v4l2_subdev_selection sel = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> + struct v4l2_subdev_format format = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> const struct v4l2_rect *crop;
> int ret;
>
> @@ -129,8 +133,6 @@ static int vsp1_du_pipeline_setup_rpf(struct vsp1_device *vsp1,
> */
> crop = &vsp1->drm->inputs[rpf->entity.index].crop;
>
> - memset(&format, 0, sizeof(format));
> - format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> format.pad = RWPF_PAD_SINK;
> format.format.width = crop->width + crop->left;
> format.format.height = crop->height + crop->top;
> @@ -147,8 +149,6 @@ static int vsp1_du_pipeline_setup_rpf(struct vsp1_device *vsp1,
> __func__, format.format.width, format.format.height,
> format.format.code, rpf->entity.index);
>
> - memset(&sel, 0, sizeof(sel));
> - sel.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> sel.pad = RWPF_PAD_SINK;
> sel.target = V4L2_SEL_TGT_CROP;
> sel.r = *crop;
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.c b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
> index 4c3bd2b1ca28..c31f05a80bb5 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_entity.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
> @@ -184,15 +184,14 @@ vsp1_entity_get_pad_selection(struct vsp1_entity *entity,
> int vsp1_entity_init_cfg(struct v4l2_subdev *subdev,
> struct v4l2_subdev_state *sd_state)
> {
> - struct v4l2_subdev_format format;
> unsigned int pad;
>
> for (pad = 0; pad < subdev->entity.num_pads - 1; ++pad) {
> - memset(&format, 0, sizeof(format));
> -
> - format.pad = pad;
> - format.which = sd_state ? V4L2_SUBDEV_FORMAT_TRY
> - : V4L2_SUBDEV_FORMAT_ACTIVE;
> + struct v4l2_subdev_format format = {
> + .pad = pad,
> + .which = sd_state ? V4L2_SUBDEV_FORMAT_TRY
> + : V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
>
> v4l2_subdev_call(subdev, pad, set_fmt, sd_state, &format);
> }
> diff --git a/drivers/media/platform/samsung/exynos4-is/fimc-capture.c b/drivers/media/platform/samsung/exynos4-is/fimc-capture.c
> index 4800751a401c..a0d43bf892e6 100644
> --- a/drivers/media/platform/samsung/exynos4-is/fimc-capture.c
> +++ b/drivers/media/platform/samsung/exynos4-is/fimc-capture.c
> @@ -763,7 +763,10 @@ static int fimc_pipeline_try_format(struct fimc_ctx *ctx,
> struct fimc_dev *fimc = ctx->fimc_dev;
> struct fimc_pipeline *p = to_fimc_pipeline(fimc->vid_cap.ve.pipe);
> struct v4l2_subdev *sd = p->subdevs[IDX_SENSOR];
> - struct v4l2_subdev_format sfmt;
> + struct v4l2_subdev_format sfmt = {
> + .which = set ? V4L2_SUBDEV_FORMAT_ACTIVE
> + : V4L2_SUBDEV_FORMAT_TRY,
> + };
> struct v4l2_mbus_framefmt *mf = &sfmt.format;
> struct media_entity *me;
> struct fimc_fmt *ffmt;
> @@ -774,9 +777,7 @@ static int fimc_pipeline_try_format(struct fimc_ctx *ctx,
> if (WARN_ON(!sd || !tfmt))
> return -EINVAL;
>
> - memset(&sfmt, 0, sizeof(sfmt));
> sfmt.format = *tfmt;
> - sfmt.which = set ? V4L2_SUBDEV_FORMAT_ACTIVE : V4L2_SUBDEV_FORMAT_TRY;
>
> me = fimc_pipeline_get_head(&sd->entity);
>
> diff --git a/drivers/media/platform/ti/am437x/am437x-vpfe.c b/drivers/media/platform/ti/am437x/am437x-vpfe.c
> index f23085b5954b..9fcbf2a2b7c3 100644
> --- a/drivers/media/platform/ti/am437x/am437x-vpfe.c
> +++ b/drivers/media/platform/ti/am437x/am437x-vpfe.c
> @@ -1501,7 +1501,9 @@ static int vpfe_enum_size(struct file *file, void *priv,
> struct v4l2_frmsizeenum *fsize)
> {
> struct vpfe_device *vpfe = video_drvdata(file);
> - struct v4l2_subdev_frame_size_enum fse;
> + struct v4l2_subdev_frame_size_enum fse = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> struct v4l2_subdev *sd = vpfe->current_subdev->sd;
> struct vpfe_fmt *fmt;
> int ret;
> @@ -1516,11 +1518,9 @@ static int vpfe_enum_size(struct file *file, void *priv,
>
> memset(fsize->reserved, 0x0, sizeof(fsize->reserved));
>
> - memset(&fse, 0x0, sizeof(fse));
> fse.index = fsize->index;
> fse.pad = 0;
> fse.code = fmt->code;
> - fse.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> ret = v4l2_subdev_call(sd, pad, enum_frame_size, NULL, &fse);
> if (ret)
> return ret;
> @@ -2148,7 +2148,6 @@ vpfe_async_bound(struct v4l2_async_notifier *notifier,
> {
> struct vpfe_device *vpfe = container_of(notifier->v4l2_dev,
> struct vpfe_device, v4l2_dev);
> - struct v4l2_subdev_mbus_code_enum mbus_code;
> struct vpfe_subdev_info *sdinfo;
> struct vpfe_fmt *fmt;
> int ret = 0;
> @@ -2175,9 +2174,11 @@ vpfe_async_bound(struct v4l2_async_notifier *notifier,
>
> vpfe->num_active_fmt = 0;
> for (j = 0, i = 0; (ret != -EINVAL); ++j) {
> - memset(&mbus_code, 0, sizeof(mbus_code));
> - mbus_code.index = j;
> - mbus_code.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> + struct v4l2_subdev_mbus_code_enum mbus_code = {
> + .index = j,
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> +
> ret = v4l2_subdev_call(subdev, pad, enum_mbus_code,
> NULL, &mbus_code);
> if (ret)
> diff --git a/drivers/media/platform/ti/cal/cal-video.c b/drivers/media/platform/ti/cal/cal-video.c
> index 51ed82172d71..ca906a9e4222 100644
> --- a/drivers/media/platform/ti/cal/cal-video.c
> +++ b/drivers/media/platform/ti/cal/cal-video.c
> @@ -814,7 +814,6 @@ static const struct v4l2_file_operations cal_fops = {
>
> static int cal_ctx_v4l2_init_formats(struct cal_ctx *ctx)
> {
> - struct v4l2_subdev_mbus_code_enum mbus_code;
> struct v4l2_mbus_framefmt mbus_fmt;
> const struct cal_format_info *fmtinfo;
> unsigned int i, j, k;
> @@ -829,10 +828,11 @@ static int cal_ctx_v4l2_init_formats(struct cal_ctx *ctx)
> ctx->num_active_fmt = 0;
>
> for (j = 0, i = 0; ; ++j) {
> + struct v4l2_subdev_mbus_code_enum mbus_code = {
> + .index = j,
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
>
> - memset(&mbus_code, 0, sizeof(mbus_code));
> - mbus_code.index = j;
> - mbus_code.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> ret = v4l2_subdev_call(ctx->phy->source, pad, enum_mbus_code,
> NULL, &mbus_code);
> if (ret == -EINVAL)
> diff --git a/drivers/media/usb/dvb-usb/cxusb-analog.c b/drivers/media/usb/dvb-usb/cxusb-analog.c
> index e93183ddd797..deba5224cb8d 100644
> --- a/drivers/media/usb/dvb-usb/cxusb-analog.c
> +++ b/drivers/media/usb/dvb-usb/cxusb-analog.c
> @@ -1014,7 +1014,10 @@ static int cxusb_medion_try_s_fmt_vid_cap(struct file *file,
> {
> struct dvb_usb_device *dvbdev = video_drvdata(file);
> struct cxusb_medion_dev *cxdev = dvbdev->priv;
> - struct v4l2_subdev_format subfmt;
> + struct v4l2_subdev_format subfmt = {
> + .which = isset ? V4L2_SUBDEV_FORMAT_ACTIVE :
> + V4L2_SUBDEV_FORMAT_TRY,
> + };
> u32 field;
> int ret;
>
> @@ -1024,9 +1027,6 @@ static int cxusb_medion_try_s_fmt_vid_cap(struct file *file,
> field = vb2_start_streaming_called(&cxdev->videoqueue) ?
> cxdev->field_order : cxusb_medion_field_order(cxdev);
>
> - memset(&subfmt, 0, sizeof(subfmt));
> - subfmt.which = isset ? V4L2_SUBDEV_FORMAT_ACTIVE :
> - V4L2_SUBDEV_FORMAT_TRY;
> subfmt.format.width = f->fmt.pix.width & ~1;
> subfmt.format.height = f->fmt.pix.height & ~1;
> subfmt.format.code = MEDIA_BUS_FMT_FIXED;
> @@ -1464,7 +1464,9 @@ int cxusb_medion_analog_init(struct dvb_usb_device *dvbdev)
> .buf = tuner_analog_msg_data,
> .len =
> sizeof(tuner_analog_msg_data) };
> - struct v4l2_subdev_format subfmt;
> + struct v4l2_subdev_format subfmt = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> int ret;
>
> /* switch tuner to analog mode so IF demod will become accessible */
> @@ -1507,8 +1509,6 @@ int cxusb_medion_analog_init(struct dvb_usb_device *dvbdev)
> v4l2_subdev_call(cxdev->tuner, video, s_std, cxdev->norm);
> v4l2_subdev_call(cxdev->cx25840, video, s_std, cxdev->norm);
>
> - memset(&subfmt, 0, sizeof(subfmt));
> - subfmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> subfmt.format.width = cxdev->width;
> subfmt.format.height = cxdev->height;
> subfmt.format.code = MEDIA_BUS_FMT_FIXED;
> diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
> index 39666db59c4e..4364df27c6d2 100644
> --- a/drivers/staging/media/imx/imx-media-capture.c
> +++ b/drivers/staging/media/imx/imx-media-capture.c
> @@ -504,14 +504,14 @@ static int capture_legacy_g_parm(struct file *file, void *fh,
> struct v4l2_streamparm *a)
> {
> struct capture_priv *priv = video_drvdata(file);
> - struct v4l2_subdev_frame_interval fi;
> + struct v4l2_subdev_frame_interval fi = {
> + .pad = priv->src_sd_pad,
> + };
> int ret;
>
> if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> return -EINVAL;
>
> - memset(&fi, 0, sizeof(fi));
> - fi.pad = priv->src_sd_pad;
> ret = v4l2_subdev_call(priv->src_sd, video, g_frame_interval, &fi);
> if (ret < 0)
> return ret;
> @@ -526,14 +526,14 @@ static int capture_legacy_s_parm(struct file *file, void *fh,
> struct v4l2_streamparm *a)
> {
> struct capture_priv *priv = video_drvdata(file);
> - struct v4l2_subdev_frame_interval fi;
> + struct v4l2_subdev_frame_interval fi = {
> + .pad = priv->src_sd_pad,
> + };
> int ret;
>
> if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> return -EINVAL;
>
> - memset(&fi, 0, sizeof(fi));
> - fi.pad = priv->src_sd_pad;
> fi.interval = a->parm.capture.timeperframe;
> ret = v4l2_subdev_call(priv->src_sd, video, s_frame_interval, &fi);
> if (ret < 0)
> diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
> index 411e907b68eb..b545750ca526 100644
> --- a/drivers/staging/media/imx/imx-media-utils.c
> +++ b/drivers/staging/media/imx/imx-media-utils.c
> @@ -432,15 +432,15 @@ int imx_media_init_cfg(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *sd_state)
> {
> struct v4l2_mbus_framefmt *mf_try;
> - struct v4l2_subdev_format format;
> unsigned int pad;
> int ret;
>
> for (pad = 0; pad < sd->entity.num_pads; pad++) {
> - memset(&format, 0, sizeof(format));
> + struct v4l2_subdev_format format = {
> + .pad = pad,
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
>
> - format.pad = pad;
> - format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &format);
> if (ret)
> continue;
> diff --git a/drivers/staging/media/omap4iss/iss_video.c b/drivers/staging/media/omap4iss/iss_video.c
> index 1406445a30c3..22fa4d6cae10 100644
> --- a/drivers/staging/media/omap4iss/iss_video.c
> +++ b/drivers/staging/media/omap4iss/iss_video.c
> @@ -237,7 +237,9 @@ static int
> __iss_video_get_format(struct iss_video *video,
> struct v4l2_mbus_framefmt *format)
> {
> - struct v4l2_subdev_format fmt;
> + struct v4l2_subdev_format fmt = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> struct v4l2_subdev *subdev;
> u32 pad;
> int ret;
> @@ -246,9 +248,7 @@ __iss_video_get_format(struct iss_video *video,
> if (!subdev)
> return -EINVAL;
>
> - memset(&fmt, 0, sizeof(fmt));
> fmt.pad = pad;
> - fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
>
> mutex_lock(&video->mutex);
> ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);
> --
> Regards,
>
> Laurent Pinchart
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] media: Zero-initialize all structures passed to subdev pad operations
2023-02-15 16:50 ` [PATCH 1/3] media: Zero-initialize all structures passed to subdev pad operations Laurent Pinchart
2023-02-27 22:30 ` Shuah Khan
2023-02-28 9:05 ` Lad, Prabhakar
@ 2023-02-28 10:05 ` Sakari Ailus
2023-02-28 23:55 ` Laurent Pinchart
2023-03-06 13:00 ` Philipp Zabel
3 siblings, 1 reply; 16+ messages in thread
From: Sakari Ailus @ 2023-02-28 10:05 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Hans Verkuil, Tomi Valkeinen, Yong Zhi, Bingbu Cao,
Dan Scally, Tianshu Qiu, Eugen Hristev, Robert Foss, Todor Tomov,
Kieran Bingham, Sylwester Nawrocki, Prabhakar Lad, Benoit Parrot,
Shuah Khan, Michael Krufky, Steve Longerbeam, Philipp Zabel,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni, kernel,
linux-imx
Hi Laurent,
Thanks for the set.
On Wed, Feb 15, 2023 at 06:50:19PM +0200, Laurent Pinchart wrote:
> Several drivers call subdev pad operations, passing structures that are
> not fully zeroed. While the drivers initialize the fields they care
> about explicitly, this results in reserved fields having uninitialized
> values. Future kernel API changes that make use of those fields thus
> risk breaking proper driver operation in ways that could be hard to
> detect.
>
> To avoid this, make the code more robust by zero-initializing all the
> structures passed to subdev pad operation. Maintain a consistent coding
> style by preferring designated initializers (which zero-initialize all
> the fields that are not specified) over memset() where possible, and
> make variable declarations local to inner scopes where applicable. One
> notable exception to this rule is in the ipu3 driver, where a memset()
> is needed as the structure is not a local variable but a function
> parameter provided by the caller.
>
> Not all fields of those structures can be initialized when declaring the
> variables, as the values for those fields are computed later in the
> code. Initialize the 'which' field in all cases, and other fields when
> the variable declaration is so close to the v4l2_subdev_call() call that
> it keeps all the context easily visible when reading the code, to avoid
> hindering readability.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
...
> diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
> index 3b76a9d0383a..3c84cb121632 100644
> --- a/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
> +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
> @@ -1305,6 +1305,7 @@ static int cio2_subdev_link_validate_get_format(struct media_pad *pad,
> struct v4l2_subdev *sd =
> media_entity_to_v4l2_subdev(pad->entity);
>
> + memset(fmt, 0, sizeof(*fmt));
> fmt->which = V4L2_SUBDEV_FORMAT_ACTIVE;
> fmt->pad = pad->index;
> return v4l2_subdev_call(sd, pad, get_fmt, NULL, fmt);
Instead I'd merge this with its only caller.
I can submit a patch on top of this one as it's just a small cleanup.
For the set:
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
The second latter of the subject of the 3 patch should be lower case.
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] media: Zero-initialize structures passed to subdev pad ops
2023-02-15 16:50 [PATCH 0/3] media: Zero-initialize structures passed to subdev pad ops Laurent Pinchart
` (2 preceding siblings ...)
2023-02-15 16:50 ` [PATCH 3/3] media: USe designated initializers for all " Laurent Pinchart
@ 2023-02-28 15:52 ` Tomi Valkeinen
2023-02-28 16:41 ` Kieran Bingham
4 siblings, 0 replies; 16+ messages in thread
From: Tomi Valkeinen @ 2023-02-28 15:52 UTC (permalink / raw)
To: Laurent Pinchart, linux-media
Cc: Hans Verkuil, Sakari Ailus, Yong Zhi, Bingbu Cao, Dan Scally,
Tianshu Qiu, Eugen Hristev, Robert Foss, Todor Tomov,
Kieran Bingham, Sylwester Nawrocki, Prabhakar Lad, Benoit Parrot,
Shuah Khan, Michael Krufky, Steve Longerbeam, Philipp Zabel,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni, kernel,
linux-imx
On 15/02/2023 18:50, Laurent Pinchart wrote:
> Hello,
>
> This patch series fixes a (surprisingly large) number of drivers that
> don't zero-initialize structures passed to subdev pad operations.
>
> The rationale is explained in patch 1/3, which fixes the issue: while
> this doesn't cause any immediate problem, it leaves reserved fields
> uninitialized, and any future change of in-kernel APIs that make use of
> some of the reserved fields may introduce hard to catch breakages.
>
> Patches 2/3 and 3/3 are not strictly required to fix the problem, but
> they address coding style consistency issues that bothered me when
> developing 1/3.
>
> Laurent Pinchart (3):
> media: Zero-initialize all structures passed to subdev pad operations
> media: Prefer designated initializers over memset for subdev pad ops
> media: USe designated initializers for all subdev pad ops
For the three patches:
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Tomi
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] media: Zero-initialize structures passed to subdev pad ops
2023-02-15 16:50 [PATCH 0/3] media: Zero-initialize structures passed to subdev pad ops Laurent Pinchart
` (3 preceding siblings ...)
2023-02-28 15:52 ` [PATCH 0/3] media: Zero-initialize structures passed to " Tomi Valkeinen
@ 2023-02-28 16:41 ` Kieran Bingham
4 siblings, 0 replies; 16+ messages in thread
From: Kieran Bingham @ 2023-02-28 16:41 UTC (permalink / raw)
To: Laurent Pinchart, linux-media
Cc: Hans Verkuil, Sakari Ailus, Tomi Valkeinen, Yong Zhi, Bingbu Cao,
Dan Scally, Tianshu Qiu, Eugen Hristev, Robert Foss, Todor Tomov,
Sylwester Nawrocki, Prabhakar Lad, Benoit Parrot, Shuah Khan,
Michael Krufky, Steve Longerbeam, Philipp Zabel, Thierry Reding,
Jonathan Hunter, Sowjanya Komatineni, kernel, linux-imx
Hi Laurent,
Quoting Laurent Pinchart (2023-02-15 16:50:18)
> Hello,
>
> This patch series fixes a (surprisingly large) number of drivers that
> don't zero-initialize structures passed to subdev pad operations.
>
> The rationale is explained in patch 1/3, which fixes the issue: while
> this doesn't cause any immediate problem, it leaves reserved fields
> uninitialized, and any future change of in-kernel APIs that make use of
> some of the reserved fields may introduce hard to catch breakages.
>
> Patches 2/3 and 3/3 are not strictly required to fix the problem, but
> they address coding style consistency issues that bothered me when
> developing 1/3.
For the series,
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>
> Laurent Pinchart (3):
> media: Zero-initialize all structures passed to subdev pad operations
> media: Prefer designated initializers over memset for subdev pad ops
> media: USe designated initializers for all subdev pad ops
>
> drivers/media/pci/cobalt/cobalt-v4l2.c | 21 ++++++----
> drivers/media/pci/intel/ipu3/ipu3-cio2-main.c | 1 +
> .../platform/microchip/microchip-isc-base.c | 5 ++-
> .../media/platform/qcom/camss/camss-video.c | 5 ++-
> .../media/platform/renesas/vsp1/vsp1_drm.c | 23 ++++++-----
> .../media/platform/renesas/vsp1/vsp1_entity.c | 11 +++--
> .../media/platform/renesas/vsp1/vsp1_video.c | 5 ++-
> .../samsung/exynos4-is/fimc-capture.c | 18 +++++----
> .../samsung/exynos4-is/fimc-isp-video.c | 10 +++--
> .../platform/samsung/exynos4-is/fimc-lite.c | 9 +++--
> .../samsung/s3c-camif/camif-capture.c | 5 ++-
> .../platform/samsung/s3c-camif/camif-core.c | 5 ++-
> .../media/platform/ti/am437x/am437x-vpfe.c | 35 ++++++++--------
> drivers/media/platform/ti/cal/cal-video.c | 37 +++++++++--------
> drivers/media/platform/ti/omap3isp/ispccdc.c | 5 ++-
> drivers/media/platform/ti/omap3isp/ispvideo.c | 20 ++++++----
> drivers/media/platform/xilinx/xilinx-dma.c | 5 ++-
> drivers/media/test-drivers/vimc/vimc-common.c | 8 ++--
> drivers/media/usb/dvb-usb/cxusb-analog.c | 14 +++----
> .../media/deprecated/atmel/atmel-isc-base.c | 5 ++-
> drivers/staging/media/imx/imx-media-capture.c | 40 ++++++++++---------
> drivers/staging/media/imx/imx-media-utils.c | 8 ++--
> drivers/staging/media/omap4iss/iss_video.c | 16 ++++----
> drivers/staging/media/tegra-video/vi.c | 10 +++--
> 24 files changed, 182 insertions(+), 139 deletions(-)
>
>
> base-commit: 83e0f265aa8d0e37cc8e15d318b64da0ec03ff41
> --
> Regards,
>
> Laurent Pinchart
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] media: Zero-initialize all structures passed to subdev pad operations
2023-02-28 10:05 ` Sakari Ailus
@ 2023-02-28 23:55 ` Laurent Pinchart
2023-02-28 23:58 ` Laurent Pinchart
0 siblings, 1 reply; 16+ messages in thread
From: Laurent Pinchart @ 2023-02-28 23:55 UTC (permalink / raw)
To: Sakari Ailus
Cc: linux-media, Hans Verkuil, Tomi Valkeinen, Yong Zhi, Bingbu Cao,
Dan Scally, Tianshu Qiu, Eugen Hristev, Robert Foss, Todor Tomov,
Kieran Bingham, Sylwester Nawrocki, Prabhakar Lad, Benoit Parrot,
Shuah Khan, Michael Krufky, Steve Longerbeam, Philipp Zabel,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni, kernel,
linux-imx
Hi Sakari,
On Tue, Feb 28, 2023 at 12:05:03PM +0200, Sakari Ailus wrote:
> On Wed, Feb 15, 2023 at 06:50:19PM +0200, Laurent Pinchart wrote:
> > Several drivers call subdev pad operations, passing structures that are
> > not fully zeroed. While the drivers initialize the fields they care
> > about explicitly, this results in reserved fields having uninitialized
> > values. Future kernel API changes that make use of those fields thus
> > risk breaking proper driver operation in ways that could be hard to
> > detect.
> >
> > To avoid this, make the code more robust by zero-initializing all the
> > structures passed to subdev pad operation. Maintain a consistent coding
> > style by preferring designated initializers (which zero-initialize all
> > the fields that are not specified) over memset() where possible, and
> > make variable declarations local to inner scopes where applicable. One
> > notable exception to this rule is in the ipu3 driver, where a memset()
> > is needed as the structure is not a local variable but a function
> > parameter provided by the caller.
> >
> > Not all fields of those structures can be initialized when declaring the
> > variables, as the values for those fields are computed later in the
> > code. Initialize the 'which' field in all cases, and other fields when
> > the variable declaration is so close to the v4l2_subdev_call() call that
> > it keeps all the context easily visible when reading the code, to avoid
> > hindering readability.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> ...
>
> > diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
> > index 3b76a9d0383a..3c84cb121632 100644
> > --- a/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
> > +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
> > @@ -1305,6 +1305,7 @@ static int cio2_subdev_link_validate_get_format(struct media_pad *pad,
> > struct v4l2_subdev *sd =
> > media_entity_to_v4l2_subdev(pad->entity);
> >
> > + memset(fmt, 0, sizeof(*fmt));
> > fmt->which = V4L2_SUBDEV_FORMAT_ACTIVE;
> > fmt->pad = pad->index;
> > return v4l2_subdev_call(sd, pad, get_fmt, NULL, fmt);
>
> Instead I'd merge this with its only caller.
>
> I can submit a patch on top of this one as it's just a small cleanup.
I'd prefer that, to keep this series as little intrusive as possible.
> For the set:
>
> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Thank you.
> The second latter of the subject of the 3 patch should be lower case.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] media: Zero-initialize all structures passed to subdev pad operations
2023-02-28 23:55 ` Laurent Pinchart
@ 2023-02-28 23:58 ` Laurent Pinchart
2023-03-01 7:55 ` Sakari Ailus
0 siblings, 1 reply; 16+ messages in thread
From: Laurent Pinchart @ 2023-02-28 23:58 UTC (permalink / raw)
To: Sakari Ailus
Cc: linux-media, Hans Verkuil, Tomi Valkeinen, Yong Zhi, Bingbu Cao,
Dan Scally, Tianshu Qiu, Eugen Hristev, Robert Foss, Todor Tomov,
Kieran Bingham, Sylwester Nawrocki, Prabhakar Lad, Benoit Parrot,
Shuah Khan, Michael Krufky, Steve Longerbeam, Philipp Zabel,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni, kernel,
linux-imx
On Wed, Mar 01, 2023 at 01:55:49AM +0200, Laurent Pinchart wrote:
> Hi Sakari,
>
> On Tue, Feb 28, 2023 at 12:05:03PM +0200, Sakari Ailus wrote:
> > On Wed, Feb 15, 2023 at 06:50:19PM +0200, Laurent Pinchart wrote:
> > > Several drivers call subdev pad operations, passing structures that are
> > > not fully zeroed. While the drivers initialize the fields they care
> > > about explicitly, this results in reserved fields having uninitialized
> > > values. Future kernel API changes that make use of those fields thus
> > > risk breaking proper driver operation in ways that could be hard to
> > > detect.
> > >
> > > To avoid this, make the code more robust by zero-initializing all the
> > > structures passed to subdev pad operation. Maintain a consistent coding
> > > style by preferring designated initializers (which zero-initialize all
> > > the fields that are not specified) over memset() where possible, and
> > > make variable declarations local to inner scopes where applicable. One
> > > notable exception to this rule is in the ipu3 driver, where a memset()
> > > is needed as the structure is not a local variable but a function
> > > parameter provided by the caller.
> > >
> > > Not all fields of those structures can be initialized when declaring the
> > > variables, as the values for those fields are computed later in the
> > > code. Initialize the 'which' field in all cases, and other fields when
> > > the variable declaration is so close to the v4l2_subdev_call() call that
> > > it keeps all the context easily visible when reading the code, to avoid
> > > hindering readability.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > ...
> >
> > > diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
> > > index 3b76a9d0383a..3c84cb121632 100644
> > > --- a/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
> > > +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
> > > @@ -1305,6 +1305,7 @@ static int cio2_subdev_link_validate_get_format(struct media_pad *pad,
> > > struct v4l2_subdev *sd =
> > > media_entity_to_v4l2_subdev(pad->entity);
> > >
> > > + memset(fmt, 0, sizeof(*fmt));
> > > fmt->which = V4L2_SUBDEV_FORMAT_ACTIVE;
> > > fmt->pad = pad->index;
> > > return v4l2_subdev_call(sd, pad, get_fmt, NULL, fmt);
> >
> > Instead I'd merge this with its only caller.
> >
> > I can submit a patch on top of this one as it's just a small cleanup.
>
> I'd prefer that, to keep this series as little intrusive as possible.
>
> > For the set:
> >
> > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>
> Thank you.
>
> > The second latter of the subject of the 3 patch should be lower case.
What ? :-)
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] media: Zero-initialize all structures passed to subdev pad operations
2023-02-28 23:58 ` Laurent Pinchart
@ 2023-03-01 7:55 ` Sakari Ailus
2023-03-01 8:43 ` Bingbu Cao
0 siblings, 1 reply; 16+ messages in thread
From: Sakari Ailus @ 2023-03-01 7:55 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Hans Verkuil, Tomi Valkeinen, Yong Zhi, Bingbu Cao,
Dan Scally, Tianshu Qiu, Eugen Hristev, Robert Foss, Todor Tomov,
Kieran Bingham, Sylwester Nawrocki, Prabhakar Lad, Benoit Parrot,
Shuah Khan, Michael Krufky, Steve Longerbeam, Philipp Zabel,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni, kernel,
linux-imx
On Wed, Mar 01, 2023 at 01:58:26AM +0200, Laurent Pinchart wrote:
> On Wed, Mar 01, 2023 at 01:55:49AM +0200, Laurent Pinchart wrote:
> > On Tue, Feb 28, 2023 at 12:05:03PM +0200, Sakari Ailus wrote:
> > > The second latter of the subject of the 3 patch should be lower case.
>
> What ? :-)
s/a/e/
--
Sakari Ailus
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] media: Zero-initialize all structures passed to subdev pad operations
2023-03-01 7:55 ` Sakari Ailus
@ 2023-03-01 8:43 ` Bingbu Cao
0 siblings, 0 replies; 16+ messages in thread
From: Bingbu Cao @ 2023-03-01 8:43 UTC (permalink / raw)
To: Sakari Ailus, Laurent Pinchart
Cc: linux-media, Hans Verkuil, Tomi Valkeinen, Yong Zhi, Bingbu Cao,
Dan Scally, Tianshu Qiu, Eugen Hristev, Robert Foss, Todor Tomov,
Kieran Bingham, Sylwester Nawrocki, Prabhakar Lad, Benoit Parrot,
Shuah Khan, Michael Krufky, Steve Longerbeam, Philipp Zabel,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni, kernel,
linux-imx
On 3/1/23 3:55 PM, Sakari Ailus wrote:
> On Wed, Mar 01, 2023 at 01:58:26AM +0200, Laurent Pinchart wrote:
>> On Wed, Mar 01, 2023 at 01:55:49AM +0200, Laurent Pinchart wrote:
>>> On Tue, Feb 28, 2023 at 12:05:03PM +0200, Sakari Ailus wrote:
>>>> The second latter of the subject of the 3 patch should be lower case.
>>
>> What ? :-)
>
> s/a/e/
:-) , s/S/s/ + s/a/e/
>
--
Best regards,
Bingbu Cao
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] media: Zero-initialize all structures passed to subdev pad operations
2023-02-15 16:50 ` [PATCH 1/3] media: Zero-initialize all structures passed to subdev pad operations Laurent Pinchart
` (2 preceding siblings ...)
2023-02-28 10:05 ` Sakari Ailus
@ 2023-03-06 13:00 ` Philipp Zabel
3 siblings, 0 replies; 16+ messages in thread
From: Philipp Zabel @ 2023-03-06 13:00 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Hans Verkuil, Sakari Ailus, Tomi Valkeinen, Yong Zhi,
Bingbu Cao, Dan Scally, Tianshu Qiu, Eugen Hristev, Robert Foss,
Todor Tomov, Kieran Bingham, Sylwester Nawrocki, Prabhakar Lad,
Benoit Parrot, Shuah Khan, Michael Krufky, Steve Longerbeam,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni, kernel,
linux-imx
On Wed, Feb 15, 2023 at 06:50:19PM +0200, Laurent Pinchart wrote:
> Several drivers call subdev pad operations, passing structures that are
> not fully zeroed. While the drivers initialize the fields they care
> about explicitly, this results in reserved fields having uninitialized
> values. Future kernel API changes that make use of those fields thus
> risk breaking proper driver operation in ways that could be hard to
> detect.
>
> To avoid this, make the code more robust by zero-initializing all the
> structures passed to subdev pad operation. Maintain a consistent coding
> style by preferring designated initializers (which zero-initialize all
> the fields that are not specified) over memset() where possible, and
> make variable declarations local to inner scopes where applicable. One
> notable exception to this rule is in the ipu3 driver, where a memset()
> is needed as the structure is not a local variable but a function
> parameter provided by the caller.
>
> Not all fields of those structures can be initialized when declaring the
> variables, as the values for those fields are computed later in the
> code. Initialize the 'which' field in all cases, and other fields when
> the variable declaration is so close to the v4l2_subdev_call() call that
> it keeps all the context easily visible when reading the code, to avoid
> hindering readability.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
[...]
> drivers/staging/media/imx/imx-media-capture.c | 28 ++++++++++--------
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
regards
Philipp
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] media: Prefer designated initializers over memset for subdev pad ops
2023-02-15 16:50 ` [PATCH 2/3] media: Prefer designated initializers over memset for subdev pad ops Laurent Pinchart
2023-02-28 9:09 ` Lad, Prabhakar
@ 2023-03-06 13:01 ` Philipp Zabel
1 sibling, 0 replies; 16+ messages in thread
From: Philipp Zabel @ 2023-03-06 13:01 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Hans Verkuil, Sakari Ailus, Tomi Valkeinen, Yong Zhi,
Bingbu Cao, Dan Scally, Tianshu Qiu, Eugen Hristev, Robert Foss,
Todor Tomov, Kieran Bingham, Sylwester Nawrocki, Prabhakar Lad,
Benoit Parrot, Shuah Khan, Michael Krufky, Steve Longerbeam,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni, kernel,
linux-imx
On Wed, Feb 15, 2023 at 06:50:20PM +0200, Laurent Pinchart wrote:
> Structures passed to subdev pad operations are all zero-initialized, but
> not always with the same kind of code constructs. While most drivers
> used designated initializers, which zero all the fields that are not
> specified, when declaring variables, some use memset(). Those two
> methods lead to the same end result, and, depending on compiler
> optimizations, may even be completely equivalent, but they're not
> consistent.
>
> Improve coding style consistency by using designated initializers
> instead of calling memset(). Where applicable, also move the variables
> to inner scopes of for loops to ensure correct initialization in all
> iterations.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
[...]
> drivers/staging/media/imx/imx-media-capture.c | 12 ++++++------
> drivers/staging/media/imx/imx-media-utils.c | 8 ++++----
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
regards
Philipp
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2023-03-06 13:02 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-15 16:50 [PATCH 0/3] media: Zero-initialize structures passed to subdev pad ops Laurent Pinchart
2023-02-15 16:50 ` [PATCH 1/3] media: Zero-initialize all structures passed to subdev pad operations Laurent Pinchart
2023-02-27 22:30 ` Shuah Khan
2023-02-28 9:05 ` Lad, Prabhakar
2023-02-28 10:05 ` Sakari Ailus
2023-02-28 23:55 ` Laurent Pinchart
2023-02-28 23:58 ` Laurent Pinchart
2023-03-01 7:55 ` Sakari Ailus
2023-03-01 8:43 ` Bingbu Cao
2023-03-06 13:00 ` Philipp Zabel
2023-02-15 16:50 ` [PATCH 2/3] media: Prefer designated initializers over memset for subdev pad ops Laurent Pinchart
2023-02-28 9:09 ` Lad, Prabhakar
2023-03-06 13:01 ` Philipp Zabel
2023-02-15 16:50 ` [PATCH 3/3] media: USe designated initializers for all " Laurent Pinchart
2023-02-28 15:52 ` [PATCH 0/3] media: Zero-initialize structures passed to " Tomi Valkeinen
2023-02-28 16:41 ` Kieran Bingham
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox