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@xxxxxxxxxxxxxxxx> > > ... > > > 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@xxxxxxxxxxxxxxx> Thank you. > The second latter of the subject of the 3 patch should be lower case. -- Regards, Laurent Pinchart
