Re: [PATCH 00/23] Memory leak fixes (pt.3)
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Aug 1, 2024, 17:16 UTC
- Message-ID
- <ZqvC1CfRj0dQkmMF@nand.local>
- In-Reply-To
- <ZqtFIblPxqoXww-p@tanuki>
On Thu, Aug 01, 2024 at 10:19:45AM +0200, Patrick Steinhardt wrote:
Show 15 quoted lines
> On Wed, Jul 31, 2024 at 01:01:07PM -0400, Taylor Blau wrote: > > On Fri, Jul 26, 2024 at 02:13:43PM +0200, Patrick Steinhardt wrote: > > > 57 files changed, 251 insertions(+), 73 deletions(-) > > > > I took a careful read through these patches, and found most of them easy > > to review. I was admittedly a little lost with the "fix various leak" > > patches, and having slightly more context in the commit descriptions > > there would have been helpful. > > Yeah, I was a bit torn here on whether to expand the commit messages or > not. I just didn't think it's all that useful to always say "Variable x > is allocated, but never freed" for trivial cases where allocation and > freeing need to happen in the same function, much less so if we repeat > such commits for every single such variable in the same subsystem. So I > just threw those fixes into a single bag.
I think sometimes that level of detail is useful, e.g., for tracking where the leak came from, if it's always existed, etc.
But I agree enumerating each leak in an otherwise straightforward commit is not a good use of author or reviewer time. I think more useful would be enumerating the kinds of leaks that you clean up.
Thanks, Taylor