On Fri, Aug 14, 2020 at 6:35 AM Jeff King <peff@xxxxxxxx> wrote: > On Thu, Aug 13, 2020 at 03:32:56PM -0400, Eric Sunshine wrote: > > 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. > > In the meantime, I have a slight preference to leave UNLEAK() there as a > potential tool for somebody digging into leak-checkers. But we almost > certainly shouldn't be asking new authors to use it in reviews, etc. I don't think it works that way in practice, though. There are enough UNLEAK()'s sprinkled around that anyone working on or around code with an existing UNLEAK() is compelled to understand/[re-]study it in order to avoid breaking existing uses and/or to correctly mirror existing uses when dealing with new resource allocations. The same applies to patches. As a reviewer, I have two choices when I see UNLEAK(): either I ignore it because I don't have the specialized knowledge in my head (which makes me feel like my review is ineffective), or I re-acquire the knowledge. And it's not just patches like the ones in this series which are actively adjusting UNLEAK() callers, but any patch which adds or removes an UNLEAK() corresponding to the central meaty changes of the patch, or even a patch in which UNLEAK() appears only in context lines, or even patches which don't contains any UNLEAK() calls, but the source file to which the patch applies does use UNLEAK(), if the reviewer consults the original source code in addition to the patch. > TBH, I'm not sure why people starting sprinkling UNLEAK() around in the > first place. ;) For the same reason that people are concerned about calling free() or otherwise releasing or unlocking resources which they have acquired: they're trying to be responsible. When a programmer sees UNLEAK() being used in or around the code being changed, he or she will attempt to maintain the fidelity of the existing code by being careful to mimic existing nearby resource handling practices.
