linux-kernel - Re: Forest Bond <forest@...ttletooquiet.net>,Greg Kroah-Hartman <gregkh@...uxfoundation.org>,devel@...verdev.osuosl.org,linux-kernel@...r.kernel.org

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-next>] [day] [month] [year] [list]

Date:   Mon, 08 Jun 2020 16:33:06 -0700
From:   Joe Perches <joe@...ches.com>
To:     Rodolfo C Villordo <rodolfovillordo@...il.com>,
        Julia Lawall <julia.lawall@...ia.fr>,
        Al Viro <viro@...iv.linux.org.uk>
Cc:     Dan Carpenter <dan.carpenter@...cle.com>,
        devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org
Subject: Re: Forest Bond <forest@...ttletooquiet.net>,Greg Kroah-Hartman
 <gregkh@...uxfoundation.org>,devel@...verdev.osuosl.org,linux-kernel@...r.kernel.org

On Mon, 2020-06-08 at 22:58 +0000, Rodolfo C Villordo wrote:
> On Mon, Jun 08, 2020 at 01:41:11AM -0700, Joe Perches wrote:
> > On Mon, 2020-06-08 at 07:59 +0200, Julia Lawall wrote:
> > > On Mon, 8 Jun 2020, Al Viro wrote:
> > > 
> > > > On Sun, Jun 07, 2020 at 10:41:56PM +0000, Rodolfo C. Villordo wrote:
> > > > > Multiple line over 80 characters fixes by splitting in multiple lines.
> > > > > Warning found by checkpatch.pl
> > > > 
> > > > I doubt that checkpatch.pl can catch the real problems there:
> > > > 
> > > > * Hungarian Notation Sucks.  Really.
> > > > * so does CamelCase, especially for wonders like s_uGetRTSCTSRsvTime
> 
> Yes, I agree with that.
> 
> > > Rodolfo,
> > > 
> > > If you work hard with Coccinelle and python scripting, it can help with
> > > the first two problems.
> > 
> > These VIA vt6655/vt6656 drivers have been in staging for more than
> > a decade.  There are relatively few checkpatch coding style
> > cleanups to do but there are many overall style issues to resolve.
> > 
> 
> Yes, vt6655/rxtx.c needs lots of work. I was avoiding submit bigger changes
> because this is my second patch submission.
> 
> Thank you all for the comments. I'm really sorry for the odd subject. 
> 
> How should I move forward with this?
> 
> 1 - Update this patch with the changes pointed by Dan Carpenter? 

Keep your changes small until you really know how this
style of linux kernel staging changes is done.

> 2 - Do a more elaborated and bigger change, like suggested by Al Viro
> and Joe Perches?

A patch series is much preferred to a single large change.

If you decide to refactor various functions, please do that
in separate, discrete patches.

Adding a #define and doing a sed like:

$ sed -i 's/(BY_AL2230_REG_LEN << 3) + IFREGCTL_REGW/AL2230_RLEN_CTL/' drivers/staging/vt6655/*.[ch]

should be a single patch.

And if you do that, another should be done for AL7230

$ sed -i 's/(BY_AL7230_REG_LEN << 3) + IFREGCTL_REGW/AL7230_RLEN_CTL/' drivers/staging/vt6655/*.[ch]

etc...

Maybe the #define BY_AL2230_REG_LEN should be 0x17 so that
the << 3 is more obviously constrained to the low byte

Maybe the + uses in the macros should be bitwise |.

Go wild after you figure out the process, just keep your
patches to obvious, small and verifiable changes.