netdev - Re: [PATCH net-next] net: dsa: mv88e6xxx: cosmetic fix

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]

Message-ID: <38346eab-9a83-12e6-ed87-53bcfae1e587@gmail.com>
Date:   Fri, 19 Mar 2021 17:46:33 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Marek Behún <kabel@...nel.org>
Cc:     Vladimir Oltean <olteanv@...il.com>, netdev@...r.kernel.org,
        davem@...emloft.net, kuba@...nel.org
Subject: Re: [PATCH net-next] net: dsa: mv88e6xxx: cosmetic fix



On 3/19/2021 3:54 PM, Marek Behún wrote:
> On Fri, 19 Mar 2021 15:14:52 -0700
> Florian Fainelli <f.fainelli@...il.com> wrote:
> 
>> On 3/19/2021 12:47 PM, Marek Behún wrote:
>>> On Fri, 19 Mar 2021 20:58:20 +0200
>>> Vladimir Oltean <olteanv@...il.com> wrote:
>>>   
>>>> On Fri, Mar 19, 2021 at 03:31:49PM +0100, Marek Behún wrote:  
>>>>> We know that the `lane == MV88E6393X_PORT0_LANE`, so we can pass `lane`
>>>>> to mv88e6390_serdes_read() instead of MV88E6393X_PORT0_LANE.
>>>>>
>>>>> All other occurances in this function are using the `lane` variable.
>>>>>
>>>>> It seems I forgot to change it at this one place after refactoring.
>>>>>
>>>>> Signed-off-by: Marek Behún <kabel@...nel.org>
>>>>> Fixes: de776d0d316f7 ("net: dsa: mv88e6xxx: add support for ...")
>>>>> ---    
>>>>
>>>> Either do the Fixes tag according to the documented convention:
>>>> git show de776d0d316f7 --pretty=fixes  
>>>
>>> THX, did not know about this.
>>>   
>>>> Fixes: de776d0d316f ("net: dsa: mv88e6xxx: add support for mv88e6393x family")
>>>>
>>>> or even better, drop it.  
>>>
>>> Why better to drop it?  
>>
>> To differentiate an essential/functional fix from a cosmetic fix. If all
>> cosmetic fixes got Fixes: tag that would get out of hands quickly.
> 
> IMO in this case the Fixes tag is not necessary beacuse the base commit
> is not in any stable kernel yet.

This is not necessarily an argument that I would use, even if the commit
you are fixing is only in net-next, when it is a functional, and the
emphasis on the functional aspect of the code, providing a Fixes: tag is
really nice as it allows people that do backports or else to identify
the commits as an ensemble.

> 
> But if the base commit was in a stable kernel already, and this
> cosmetic fix was sent into net-next / net, I think the Fixes tag should
> be there, in order for it to get applied into stable releases so that
> future fixes could be applied cleanly.
> 
> Or am I wrong? This is how I understand this whole system...

Your reasoning is not wrong, for cosmetic changes that do not result in
functional changes, I would say that the Fixes: is optional.
-- 
Florian