Re: [PATCH net-next] sctp: Remove commented out code
From: Thorsten Blum
Date: Thu Feb 13 2025 - 05:50:18 EST
Hi Jakub,
> On 13. Feb 2025, at 04:57, Jakub Kicinski wrote:
> On Tue, 11 Feb 2025 12:33:57 +0100 Mateusz Polchlopek wrote:
>>>> I don't think we want to remove that piece of code, please refer
>>>> to the discussion under the link:
>>>>
>>>> https://lore.kernel.org/netdev/cover.1681917361.git.lucien.xin@xxxxxxxxx/
>>>
>>> Hm, the commit message (dbda0fba7a14) says payload was deleted because
>>> "the member is not even used anywhere," but it was just commented out.
>>> In the cover letter it then explains that "deleted" actually means
>>> "commented out."
>>>
>>> However, I can't follow the reasoning in the cover letter either:
>>>
>>> "Note that instead of completely deleting it, we just leave it as a
>>> comment in the struct, signalling to the reader that we do expect
>>> such variable parameters over there, as Marcelo suggested."
>>>
>>> Where do I find Marcelo's suggestion and the "variable parameters over
>>> there?"
>>>
>>
>> That's good question, I can't find the Marcelo suggestion that author
>> mention. It's hard to find without links to previous series or
>> discussion :/
>>
>> I guess it should be also commented by maintainers, I see that in the
>> Xin's thread Kuba also commented change with commenting out instead
>> of removing code. Let's wait
>
> In the linked thread the point was to document what struct will be next
> in memory. Here we'd be leaving an array of u8s which isn't very
> informative. I see there's precedent in this file, but I vote we just
> delete the line.
This patch deletes the line and I'm wondering why the "cr"?
Were you referring to this patch maybe?
https://lore.kernel.org/r/20250114215439.916207-3-thorsten.blum@xxxxxxxxx/
Should both payload fields just be deleted since they're not used?
Thanks,
Thorsten