netdev - Re: [PATCH net-next V3 04/11] net/mlx5: fs, add mlx5_fs

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: <e241a7a4-1128-44a2-ad9f-1d5424c86e22@intel.com>
Date: Thu, 19 Dec 2024 16:36:44 +0100
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Moshe Shemesh <moshe@...dia.com>
CC: <netdev@...r.kernel.org>, Saeed Mahameed <saeedm@...dia.com>, Gal Pressman
	<gal@...dia.com>, Leon Romanovsky <leonro@...dia.com>, Mark Bloch
	<mbloch@...dia.com>, Yevgeny Kliteynik <kliteyn@...dia.com>, "David S.
 Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>, Eric Dumazet <edumazet@...gle.com>, Andrew Lunn
	<andrew+netdev@...n.ch>, Tariq Toukan <tariqt@...dia.com>, ITP Upstream
	<nxne.cnse.osdt.itp.upstreaming@...el.com>
Subject: Re: [PATCH net-next V3 04/11] net/mlx5: fs, add mlx5_fs_pool API

On 12/19/24 13:30, Moshe Shemesh wrote:
> 
> 
> On 12/19/2024 11:17 AM, Przemek Kitszel wrote:
>>
>> On 12/18/24 16:09, Tariq Toukan wrote:
>>> From: Moshe Shemesh <moshe@...dia.com>


>>> +     if (mlx5_fs_bulk_init(dev, &fc_bulk->fs_bulk, bulk_len))
>>> +             goto err_fs_bulk_init;
>>
>> Locally (say two lines above) your label name is obvious.
>> But please imagine it in the context of whole function, it is much
>> better to name labels after what they jump to (instead of what they
>> jump from). It is not only easier to reason about, but also more
>> future proof. I think Simon would agree.
>> I'm fine with keeping existing code as-is, but for new code, it's
>> always better to write it up to the best practices known.
>>
> 
> I tend to name labels according to what they jump from. Though if I see 
> on same function labels are used the other way I try to be consistent 
> with current code.
> I think there are pros and cons for both ways and both ways are used.
> I can change here, but is that kernel or netdev consensus ?

Would be great, but do we really need to open naming things for dispute/
call for moderation? (To give even more trival example: I always push
people to not name variable "status" when "err" is better, should that
be in the official doc? xD)

(to be clear - I'm fine with setting "the rule" for new code, to don't
mandate change for things already in-flight)

--
Re convincing: I would say that "came from" labels have almost no
benefits, and according to wikipedia, people make fun of them since '70
https://en.wikipedia.org/wiki/COMEFROM

It's hard to make a good came-from name for a label that needs to be
jumped-to multiple times.
Such labels are also disconnected from the code under them.

On the plus side I see only "a bit smaller diff" in some cases. But that
hurts when you just move a call with it's error handling, because, due
to name, it looks fine in the new place, but it obviously isn't.