linux-kernel - Re: [PATCH v2 0/7] drop some unnecessary wrappers

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]

Message-ID: <3484215.R56niFO833@leap>
Date:   Tue, 12 Apr 2022 11:53:42 +0200
From:   "Fabio M. De Francesco" <fmdefrancesco@...il.com>
To:     Dan Carpenter <dan.carpenter@...cle.com>
Cc:     Larry Finger <Larry.Finger@...inger.net>,
        Phillip Potter <phil@...lpotter.co.uk>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Michael Straube <straube.linux@...il.com>,
        Vihas Makwana <makvihas@...il.com>,
        linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org,
        Pavel Skripkin <paskripkin@...il.com>
Subject: Re: [PATCH v2 0/7] drop some unnecessary wrappers

On martedì 12 aprile 2022 07:06:30 CEST Dan Carpenter wrote:
> On Mon, Apr 11, 2022 at 10:34:44PM +0200, Fabio M. De Francesco wrote:
> > On lunedì 11 aprile 2022 12:21:29 CEST Vihas Makwana wrote:
> > > Drop some unnecessary wrappers and update all the references
> > > accordingly.
> > > Tested on Comfast CF-WU810N RTL8188EUS wireless adapter.
> > > 
> > > v1 -> v2:
> > > Drop the wrapper functions with underscores prefixed.
> > > 
> > > Vihas Makwana (7):
> > >   staging: r8188eu: drop unnecessary wrapper _rtw_free_cmd_priv
> > >   staging: r8188eu: drop unnecessary wrapper _rtw_init_cmd_priv
> > >   staging: r8188eu: drop unnecessary wrapper _rtw_init_evt_priv
> > >   staging: r8188eu: drop unnecessary wrapper _rtw_init_mlme_priv
> > >   staging: r8188eu: drop unnecessary wrapper _rtw_free_mlme_priv
> > >   staging: r8188eu: drop unnecessary wrapper _rtw_alloc_network
> > >   staging: r8188eu: drop unnecessary wrapper _rtw_dequeue_cmd
> > > 
> > >  drivers/staging/r8188eu/core/rtw_cmd.c     | 145 +++++++----------
> > >  drivers/staging/r8188eu/core/rtw_mlme.c    | 179 +++++++++------------
> > >  drivers/staging/r8188eu/include/rtw_mlme.h |   4 +-
> > >  3 files changed, 135 insertions(+), 193 deletions(-)
> > > 
> > > -- 
> > > 2.30.2
> > > 
> > Formally, you are removing the wrapped functions (or helpers, if you 
> > prefer) by moving their code into the wrappers. To say that you are 
> > removing the wrappers is not correct.
> 
> I once had someone make me re-write a commit message four time just as a
> kind of bullying and then at the end he was like, "You said NULL
> dereference instead of NULL pointer dereference so I had to re-write the
> commit message and I added some comment to the kernel git log explaining
> how you suck."  
> So these days I have made it a rule that if you're going
> to complain about commit messages then you have to write your own for
> people to cut and paste.  Otherwise people are like, "You're too stupid
> to read my mind.  LOL.  Do it again."

I'm assuming you don't believe that I attempted some kind of bullying similar
to what you had to face when you sent the above-mentioned patch.

Nevertheless, I was expecting some sort of reaction from you but I think 
that you are misunderstanding what my intentions were.

I didn't suggest a re-write of the commit messages. I just pointed out that 
those messages are formally inaccurate but that these kinds of small formal 
inaccuracies won't ever prevent Vihas' patches from being accepted as they are.

It's only that, IMO, if people start to take care of being formally correct 
in drivers/staging, even when they do simple changes, they train their minds
to improve their abilities to communicate in the future when they eventually
decide to submit much more complex patches in other subsystems.

> But in this case the commit message is fine.  The key things with a
> commit message are:
> 
> 1) What's the motivation
> 2) What's the effect for the user
> 3) Are all the surprising aspects are explained.  Do I have enough
>    information to review it quickly.
> 
> Removing wrappers is the motivation.  No need to explain that further.
> No effects for the user.
> There were no surprising bits.
> 
> It's fine.

I agree with you: "it's fine". I respect your opinion and your pragmatism:
after all, here, everything is pretty clear, so why bother?

However, writing accurate and formally correct commit messages takes the 
same amount of time of writing inaccurate messages. So, why not do better 
next time?

Again, I don't suggest to do so now. Not for these patches, because it would 
only be a loss of precious time.

Thanks,

Fabio M. De Francesco 

> 
> regards,
> dan carpenter
> 
>