Re: [PATCH 00/23] Memory leak fixes (pt.3)

[Taylor Blau <me@ttaylorr.com>]

On Thu, Aug 01, 2024 at 10:19:45AM +0200, Patrick Steinhardt wrote:
> 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