netdev - Re: [PATCH net] net: bcmasp: fix memory leak when bringing down if

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: <3a5cb80e-7169-4e82-b10c-843ff1eb0fd3@broadcom.com>
Date: Wed, 17 Apr 2024 09:52:47 -0700
From: Florian Fainelli <florian.fainelli@...adcom.com>
To: Simon Horman <horms@...nel.org>, Markus Elfring <Markus.Elfring@....de>
Cc: Justin Chen <justin.chen@...adcom.com>,
 bcm-kernel-feedback-list@...adcom.com, netdev@...r.kernel.org,
 kernel-janitors@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Florian Fainelli <florian.fainelli@...adcom.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH net] net: bcmasp: fix memory leak when bringing down if

On 4/17/24 09:19, Simon Horman wrote:
> On Mon, Apr 15, 2024 at 09:46:44PM +0200, Markus Elfring wrote:
>>>>> When bringing down the TX rings we flush the rings but forget to
>>>>> reclaimed the flushed packets. This lead to a memory leak since we
>>>>> do not free the dma mapped buffers. …
>>>>
>>>> I find this change description improvable.
>>>>
>>>> * How do you think about to avoid typos?
>>>>
>>>> * Would another imperative wording be more desirable?
>>>
>>> The change description makes sense to me. Can you be a bit more specific as to what isn't clear here?
>>
>> Spelling suggestions:
>> + … forget to reclaim …
>> + … This leads to …
> 
> Markus, let's cut to the chase.
> 
> What portion of your responses of this thread were produced
> by an LLM or similar technology?
> 
> The suggestions in your second email are correct.
> But, ironically, your first response appears to be grammatically incorrect.
> 
> Specifically:
> 
> * What does "improvable" mean in this context?

I read it as "improbable", but this patch came out of an actual bug 
report we had internally and code inspection revealed the leaks being 
plugged by this patch.

> * "How do you think about to avoid typos?"
>    is, in my opinion, grammatically incorrect.
>    And, FWIW, I see no typos.

There was one, "This lead to a memory leak" -> "This leads to a memory leak"

> * "Would another imperative wording be more desirable?"
>    is, in my opinion, also grammatically incorrect.
> 
> And yet your comment is ostensibly about grammar.
> I'm sorry, but this strikes me as absurd.

Yeah, I share that too, if you are to nitpick on every single word 
someone wrote in a commit message, your responses better be squeaky 
clean such that Shakespeare himself would be proud of you.

There is a track record of what people might consider bike shedding, 
others might consider useless, and others might find uber pedantic 
comments from Markus done under his other email address: 
elfring@...rs.sourceforge.net.

Me personally, I read his comments and apply my own judgement as to 
whether they justify spinning a new patch just to address the feedback 
given. He has not landed on my ignore filter, but of course that can 
change at a moments notice.
-- 
Florian

Download attachment "smime.p7s" of type "application/pkcs7-signature" (4221 bytes)