linux-kernel - Re: [PATCH v2 8/9] media: subdev: Refactor v4l2_subdev_enable/disable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Wed, 10 Apr 2024 11:17:20 +0000
From: Sakari Ailus <sakari.ailus@...ux.intel.com>
To: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
Cc: Mauro Carvalho Chehab <mchehab@...nel.org>,
	Hans Verkuil <hverkuil@...all.nl>,
	Laurent Pinchart <laurent.pinchart@...asonboard.com>,
	Umang Jain <umang.jain@...asonboard.com>,
	linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 8/9] media: subdev: Refactor
 v4l2_subdev_enable/disable_streams()

On Wed, Apr 10, 2024 at 01:35:41PM +0300, Tomi Valkeinen wrote:
> On 10/04/2024 13:13, Sakari Ailus wrote:
> > Moi,
> > 
> > Thank you for working on this.
> > 
> > On Fri, Apr 05, 2024 at 12:14:26PM +0300, Tomi Valkeinen wrote:
> > > Add two internal helper functions, v4l2_subdev_collect_streams() and
> > > v4l2_subdev_set_streams_enabled(), which allows us to refactor
> > > v4l2_subdev_enable/disable_streams() functions.
> > > 
> > > This (I think) makes the code a bit easier to read, and lets us more
> > > easily add new functionality in the helper functions in the following
> > > patch.
> > > 
> > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
> > > ---
> > >   drivers/media/v4l2-core/v4l2-subdev.c | 111 +++++++++++++++++++---------------
> > >   1 file changed, 62 insertions(+), 49 deletions(-)
> > > 
> > > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> > > index 015f2fb423c9..6c3c9069f1e2 100644
> > > --- a/drivers/media/v4l2-core/v4l2-subdev.c
> > > +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> > > @@ -2099,6 +2099,44 @@ int v4l2_subdev_routing_validate(struct v4l2_subdev *sd,
> > >   }
> > >   EXPORT_SYMBOL_GPL(v4l2_subdev_routing_validate);
> > > +static void v4l2_subdev_collect_streams(struct v4l2_subdev *sd,
> > > +					struct v4l2_subdev_state *state,
> > > +					u32 pad, u64 streams_mask,
> > > +					u64 *found_streams,
> > > +					u64 *enabled_streams)
> > > +{
> > > +	*found_streams = 0;
> > > +	*enabled_streams = 0;
> > > +
> > > +	for (unsigned int i = 0; i < state->stream_configs.num_configs; ++i) {
> > > +		const struct v4l2_subdev_stream_config *cfg;
> > > +
> > > +		cfg = &state->stream_configs.configs[i];
> > 
> > You can do the assignment in declaration.
> 
> I can, but you want the lines to be split at 80, so that'll end up being in
> two lines, which, I think, looks messier than the one above.
> 
> Usually I think doing initialization when declaring the variable is best
> done if it fits into one line.

I don't consider a line break being an issue here (or almost anywhere else,
except possibly for arrays of definitions).

-- 
Sakari Ailus