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

From: Florian Fainelli
Date: Wed Apr 17 2024 - 12:53:23 EST

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@xxxxxxxxxxxxxxxxxxxxx.

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

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature