linux-kernel - Re: [PATCH v7 15/19] staging: r8188eu: clean up usbctrl

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:   Fri, 17 Sep 2021 16:44:17 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     "Fabio M. De Francesco" <fmdefrancesco@...il.com>
Cc:     Larry Finger <Larry.Finger@...inger.net>,
        Phillip Potter <phil@...lpotter.co.uk>,
        Pavel Skripkin <paskripkin@...il.com>,
        linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org,
        David Laight <david.Laight@...lab.com>,
        Dan Carpenter <dan.carpenter@...cle.com>
Subject: Re: [PATCH v7 15/19] staging: r8188eu: clean up usbctrl_vendorreq()

On Fri, Sep 17, 2021 at 09:18:33AM +0200, Fabio M. De Francesco wrote:
> Clean up usbctrl_vendoreq() in usb_ops_linux.c.
> 
> List of changes:
> 
> 1) Rename variables:
> 	pdata => data
>         pio_priv => io_priv
>         pintfhdl => intfhdl
>         wvalue => address.
> 2) Reorder variables declarations according to the "Reverse Xmas Tree"
>    style.
> 3) Remove unncecessary test for "!pIo_buf".
> 4) Move comments one line below code.
> 5) Remove unnecessary excess parentheses.
> 6) Remove unnecessary extra spaces.
> 7) Remove unnecessary comments.
> 8) Fix grammar errors (checksumed => checksummed).

When you find yourself listing all of the different things you have done
in a single commit, that is a HUGE hint that you need to break this up
into smaller pieces.

Please do so here, this should not be just one change, as it's almost
impossible to look at this and "know" it's all still the same logic
happening here.  But if you had broken this down into 8 different
changes, then it would have been obvious and I could easily have applied
the changes.

I've taken the first 14 patches in this series, it's great work, thank
you all for doing this.  But this, and the remaining patches in here
need to be split up more to make it obvious that the changes are correct
and should be accepted.  Please feel free to start the numbering of the
patch series over now, given that the first 14 are now merged into my
tree.

thanks,

greg k-h