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] [thread-next>] [day] [month] [year] [list]
Date: Mon, 18 Mar 2024 12:41:31 +0100
From: Francesco Dolcini <francesco@...cini.it>
To: David Lin <yu-hao.lin@....com>
Cc: Brian Norris <briannorris@...omium.org>,
	"linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"kvalo@...nel.org" <kvalo@...nel.org>,
	"francesco@...cini.it" <francesco@...cini.it>,
	Pete Hsieh <tsung-hsien.hsieh@....com>,
	Francesco Dolcini <francesco.dolcini@...adex.com>
Subject: Re: [PATCH v9 1/2] wifi: mwifiex: add host mlme for client mode

Hello David,

On Mon, Mar 18, 2024 at 02:00:35AM +0000, David Lin wrote:
> > From: Brian Norris <briannorris@...omium.org>
..
> > >  .../net/wireless/marvell/mwifiex/sta_ioctl.c  |   2 +-
> > >  drivers/net/wireless/marvell/mwifiex/sta_tx.c |   9 +-
> > >  drivers/net/wireless/marvell/mwifiex/util.c   |  80 +++++
> > >  15 files changed, 671 insertions(+), 14 deletions(-)
> > 
> > (Per the above, I'd normally consider whether ~671 new lines is
> > worth splitting into multiple patches. But I don't see any great
> > logical ways to do that.)
> Francesco suggested to use two patches for this host mlme new feature
> from previous many patches. I knew it is a lot of changes, but I think
> it should be the best way to add host mlme with two patches (one for
> client and one for AP).

What I explicitly asked was to not add code in a patch, and fix the
newly added code in a following patch. What you are supposed to do is to
just amend the original code when you get review feedback.

Splitting a big patch into multiple patches is welcome to easier review,
and this needs to be done breaking down in logical pieces keeping in
mind also bisect-ability.

This [1] is an example of the addition of a relatively big new driver,
and you can see that the series is broken down in smaller patches like
"Add skeleton PowerVR driver", with intermediate steps that were
non-functional, but they were building fine, they were correct and they
were enabling more effective code review.

Unfortunately, as Brian agreed here, there was no easy way to do it for
this patch.

Francesco

[1] https://lore.kernel.org/all/cover.1700668843.git.donald.robson@imgtec.com/


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ