netdev - Re: [PATCH net v2 1/6] igc: Rename qbv_enable to taprio_offload

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: Sun, 2 Jul 2023 21:17:01 +0200
From: Florian Kauer <florian.kauer@...utronix.de>
To: Markus Elfring <Markus.Elfring@....de>, intel-wired-lan@...ts.osuosl.org,
 netdev@...r.kernel.org, kernel-janitors@...r.kernel.org,
 Aravindhan Gunasekaran <aravindhan.gunasekaran@...el.com>,
 "David S . Miller" <davem@...emloft.net>, Eric Dumazet
 <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Jesse Brandeburg <jesse.brandeburg@...el.com>,
 Kurt Kanzenbach <kurt@...utronix.de>,
 Mallikarjuna Chilakala <mallikarjuna.chilakala@...el.com>,
 Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@...el.com>,
 Paolo Abeni <pabeni@...hat.com>, Tan Tee Min <tee.min.tan@...ux.intel.com>,
 Tony Nguyen <anthony.l.nguyen@...el.com>,
 Vinicius Costa Gomes <vinicius.gomes@...el.com>
Cc: LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net v2 1/6] igc: Rename qbv_enable to
 taprio_offload_enable

Hi Markus,

On 02.07.23 18:55, Markus Elfring wrote:
>> The rename should reduce this confusion.
> 
> Would the wording “Reduce this confusion by renaming a variable at three places”
> be more appropriate for a subsequent change description?
> 
> See also:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.4#n94

Thanks for pointing that out (also in your other mail for this series).
I will be more careful regarding the use of imperative mood.

> 
>>                                          Since it is a pure
>> rename, it has no impact on functionality.
>>
>> Fixes: e17090eb2494 ("igc: allow BaseTime 0 enrollment for Qbv")
> 
> How does such information fit together?

The referenced commit introduced an issue into the kernel by
introducing a variable that does not exactly describe its actual purpose.
It is not only a cosmetic change, but in my view this confusion
was related to other issues (see the other patches). So, it seemed to be worth
fixing alongside with the other fixes, even if it does not directly impact
functionality if it is applied or not (until someone else comes along,
also gets confused and introduces another bug...).

Thanks,
Florian