Re: [PATCH 0/2] UNLEAK style fixes

[Eric Sunshine <sunshine@sunshineco.com>]

On Thu, Aug 13, 2020 at 11:54 AM Jeff King <peff@peff.net> wrote:
> (As a side note, if we want to declare UNLEAK() a failure because nobody
> cares enough to really use it, I'm OK with that, too).

Perhaps the reason that UNLEAK() has not been particularly successful,
in general, is that it requires extra knowledge and reasoning to know
when to use it and how to do so properly. Couple that with the fact
that the scope of cases where it can be used is quite narrow compared
to sum total of all code in project for which we simply free resources
when we're done with them. So, it's hard to keep the specialized
UNLEAK() knowledge in one's head.

Speaking from personal experience, the several times I have had to
deal with UNLEAK(), I had to re-learn it from scratch each time. That
meant studying the header comment, studying the implementation, and
studying existing callers before things "clicked" enough to be able to
feel confident about how to use it (assuming it wasn't false
confidence).

Even today, reading this patch series, I had to go through all that
again just to understand the changes made by the patches, and
especially the commit message of patch [1/2]. It took several
re-reads, plus re-examining UNLEAK() documentation, plus looking at
the UNLEAK() implementation a couple times before the [1/2] commit
message finally "clicked".

That all represents a lot of cognitive overhead versus the common
practice of simply freeing resources when you're done with them, which
requires no extra cognitive load since it is something we think about
_always_ when working with a language like C with no built-in garbage
collection.

So, I for one would not be especially sad to see UNLEAK() retired.

(The patch series itself looked fine and made sense once I had
re-acquired the necessary UNLEAK() knowledge.)