{"thread":{"id":"54036","subject":"[PATCH 0/2] UNLEAK style fixes","startedAt":"2020-08-13T15:54:30Z","lastAt":"2020-08-14T16:23:50Z","messageCount":9,"participants":["Jeff King","Derrick Stolee","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"403562","messageId":"20200813155426.GA896769@coredump.intra.peff.net","threadId":"54036","inReplyTo":null,"subject":"[PATCH 0/2] UNLEAK style fixes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-08-13T15:54:26Z","receivedAt":"2020-08-13T15:54:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Although we introduced UNLEAK() long ago, I don't know that anybody has\nreally made a concerted effort to annotate enough variables to make\nrunning a leak-checker useful. So I haven't paid too much attention to\nits use.\n\nBut a few people have added some annotations, and I think some of them\naren't great examples. So I decided to clean them up. This by definition\nhas no impact on regular builds (since UNLEAK is a noop there), but even\nin leak-checking builds should give no behavior change.\n\nAnother category that I was tempted to change is when variables _could_\nbe freed, but we just don't bother to do so. E.g., at the end of\nbugreport.c, we have:\n\n  UNLEAK(buffer);\n  UNLEAK(report_path);\n  return !!launch_editor(report_path.buf, NULL, NULL);\n\nUsing UNLEAK(report_path) makes sense; we can't free it because we're\npassing it to a function that runs until program end. But we _could_\nfree \"buffer\" here, which isn't otherwise used again (i.e., that could\nbe strbuf_release() instead of UNLEAK).\n\nBut that does have a run-time cost (we'd actually free the memory, even\nthough we could just exit and let the OS handle it). My guess is that\nit's not a measurable cost, and the code might be cleaner to actually\nclean up instead of sprinkling more UNLEAKs around. But until we're\nactually pushing forward with a real effort to get a leak-checker\nrunning clean, I don't see much point in doing one or the other.\n\n(As a side note, if we want to declare UNLEAK() a failure because nobody\ncares enough to really use it, I'm OK with that, too).\n\n  [1/2]: stop calling UNLEAK() before die()\n  [2/2]: ls-remote: simplify UNLEAK() usage\n\n bugreport.c         | 4 +---\n builtin/ls-remote.c | 8 +++-----\n midx.c              | 8 ++------\n 3 files changed, 6 insertions(+), 14 deletions(-)\n\n-Peff\n"},{"id":"403563","messageId":"20200813155500.GA897132@coredump.intra.peff.net","threadId":"54036","inReplyTo":"20200813155426.GA896769@coredump.intra.peff.net","subject":"[PATCH 1/2] stop calling UNLEAK() before die()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-08-13T15:55:00Z","receivedAt":"2020-08-13T15:55:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The point of UNLEAK() is to make a reference to a variable that is about\nto go out of scope so that leak-checkers will consider it to be\nnot-leaked. Doing so right before die() is therefore pointless; even\nthough we are about to exit the program, the variable will still be on\nthe stack and accessible to leak-checkers.\n\nThese annotations aren't really hurting anything, but they clutter the\ncode and set a bad example of how to use UNLEAK().\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n bugreport.c | 4 +---\n midx.c      | 8 ++------\n 2 files changed, 3 insertions(+), 9 deletions(-)\n\ndiff --git a/bugreport.c b/bugreport.c\nindex 09579e268d..7ca0fba1b8 100644\n--- a/bugreport.c\n+++ b/bugreport.c\n@@ -175,10 +175,8 @@ int cmd_main(int argc, const char **argv)\n \t/* fopen doesn't offer us an O_EXCL alternative, except with glibc. */\n \treport = open(report_path.buf, O_CREAT | O_EXCL | O_WRONLY, 0666);\n \n-\tif (report < 0) {\n-\t\tUNLEAK(report_path);\n+\tif (report < 0)\n \t\tdie(_(\"couldn't create a new file at '%s'\"), report_path.buf);\n-\t}\n \n \tif (write_in_full(report, buffer.buf, buffer.len) < 0)\n \t\tdie_errno(_(\"unable to write to %s\"), report_path.buf);\ndiff --git a/midx.c b/midx.c\nindex a5fb797ede..737420f157 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -807,11 +807,9 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *\n \tint result = 0;\n \n \tmidx_name = get_midx_filename(object_dir);\n-\tif (safe_create_leading_directories(midx_name)) {\n-\t\tUNLEAK(midx_name);\n+\tif (safe_create_leading_directories(midx_name))\n \t\tdie_errno(_(\"unable to create leading directories of %s\"),\n \t\t\t  midx_name);\n-\t}\n \n \tif (m)\n \t\tpacks.m = m;\n@@ -1051,10 +1049,8 @@ void clear_midx_file(struct repository *r)\n \t\tr->objects->multi_pack_index = NULL;\n \t}\n \n-\tif (remove_path(midx)) {\n-\t\tUNLEAK(midx);\n+\tif (remove_path(midx))\n \t\tdie(_(\"failed to clear multi-pack-index at %s\"), midx);\n-\t}\n \n \tfree(midx);\n }\n-- \n2.28.0.573.gec6564704b\n\n"},{"id":"403564","messageId":"20200813155551.GB897132@coredump.intra.peff.net","threadId":"54036","inReplyTo":"20200813155426.GA896769@coredump.intra.peff.net","subject":"[PATCH 2/2] ls-remote: simplify UNLEAK() usage","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-08-13T15:55:51Z","receivedAt":"2020-08-13T15:55:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We UNLEAK() the \"sorting\" list created by parsing command-line options\n(which is essentially used until the program exits). But we do so right\nbefore leaving the cmd_ls_remote() function, which means we have to hit\nall of the exits. But the point of UNLEAK() is that it's an annotation\nwhich doesn't impact the variable itself. We can mark it as soon as\nwe're done writing its value, and then we only have to do so once.\n\nThis gives us a minor code reduction, and serves as a better example of\nhow UNLEAK() can be used.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/ls-remote.c | 8 +++-----\n 1 file changed, 3 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/ls-remote.c b/builtin/ls-remote.c\nindex ea91679f33..092917eca2 100644\n--- a/builtin/ls-remote.c\n+++ b/builtin/ls-remote.c\n@@ -83,6 +83,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \t\t\t     PARSE_OPT_STOP_AT_NON_OPTION);\n \tdest = argv[0];\n \n+\tUNLEAK(sorting);\n+\n \tif (argc > 1) {\n \t\tint i;\n \t\tpattern = xcalloc(argc, sizeof(const char *));\n@@ -107,7 +109,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \n \tif (get_url) {\n \t\tprintf(\"%s\\n\", *remote->url);\n-\t\tUNLEAK(sorting);\n \t\treturn 0;\n \t}\n \n@@ -122,10 +123,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \t\tint hash_algo = hash_algo_by_ptr(transport_get_hash_algo(transport));\n \t\trepo_set_hash_algo(the_repository, hash_algo);\n \t}\n-\tif (transport_disconnect(transport)) {\n-\t\tUNLEAK(sorting);\n+\tif (transport_disconnect(transport))\n \t\treturn 1;\n-\t}\n \n \tif (!dest && !quiet)\n \t\tfprintf(stderr, \"From %s\\n\", *remote->url);\n@@ -150,7 +149,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \t\tstatus = 0; /* we found something */\n \t}\n \n-\tUNLEAK(sorting);\n \tref_array_clear(&ref_array);\n \treturn status;\n }\n-- \n2.28.0.573.gec6564704b\n"},{"id":"403581","messageId":"275f3f6c-77ed-fddd-8f79-28f25b257362@gmail.com","threadId":"54036","inReplyTo":"20200813155500.GA897132@coredump.intra.peff.net","subject":"Re: [PATCH 1/2] stop calling UNLEAK() before die()","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-08-13T18:08:45Z","receivedAt":"2020-08-13T18:08:49Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 8/13/2020 11:55 AM, Jeff King wrote:\n> The point of UNLEAK() is to make a reference to a variable that is about\n> to go out of scope so that leak-checkers will consider it to be\n> not-leaked. Doing so right before die() is therefore pointless; even\n> though we are about to exit the program, the variable will still be on\n> the stack and accessible to leak-checkers.\n> \n> These annotations aren't really hurting anything, but they clutter the\n> code and set a bad example of how to use UNLEAK().\n\nGood justification. I'll stop being a bad example ;)\n\nThanks,\n-Stolee\n\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n>  bugreport.c | 4 +---\n>  midx.c      | 8 ++------\n>  2 files changed, 3 insertions(+), 9 deletions(-)\n> \n> diff --git a/bugreport.c b/bugreport.c\n> index 09579e268d..7ca0fba1b8 100644\n> --- a/bugreport.c\n> +++ b/bugreport.c\n> @@ -175,10 +175,8 @@ int cmd_main(int argc, const char **argv)\n>  \t/* fopen doesn't offer us an O_EXCL alternative, except with glibc. */\n>  \treport = open(report_path.buf, O_CREAT | O_EXCL | O_WRONLY, 0666);\n>  \n> -\tif (report < 0) {\n> -\t\tUNLEAK(report_path);\n> +\tif (report < 0)\n>  \t\tdie(_(\"couldn't create a new file at '%s'\"), report_path.buf);\n> -\t}\n>  \n>  \tif (write_in_full(report, buffer.buf, buffer.len) < 0)\n>  \t\tdie_errno(_(\"unable to write to %s\"), report_path.buf);\n> diff --git a/midx.c b/midx.c\n> index a5fb797ede..737420f157 100644\n> --- a/midx.c\n> +++ b/midx.c\n> @@ -807,11 +807,9 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *\n>  \tint result = 0;\n>  \n>  \tmidx_name = get_midx_filename(object_dir);\n> -\tif (safe_create_leading_directories(midx_name)) {\n> -\t\tUNLEAK(midx_name);\n> +\tif (safe_create_leading_directories(midx_name))\n>  \t\tdie_errno(_(\"unable to create leading directories of %s\"),\n>  \t\t\t  midx_name);\n> -\t}\n>  \n>  \tif (m)\n>  \t\tpacks.m = m;\n> @@ -1051,10 +1049,8 @@ void clear_midx_file(struct repository *r)\n>  \t\tr->objects->multi_pack_index = NULL;\n>  \t}\n>  \n> -\tif (remove_path(midx)) {\n> -\t\tUNLEAK(midx);\n> +\tif (remove_path(midx))\n>  \t\tdie(_(\"failed to clear multi-pack-index at %s\"), midx);\n> -\t}\n>  \n>  \tfree(midx);\n>  }\n> \n\n"},{"id":"403582","messageId":"df89c33b-87c7-8d1e-501f-f0660aecbed0@gmail.com","threadId":"54036","inReplyTo":"20200813155551.GB897132@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] ls-remote: simplify UNLEAK() usage","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-08-13T18:11:42Z","receivedAt":"2020-08-13T18:11:47Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 8/13/2020 11:55 AM, Jeff King wrote:\n> We UNLEAK() the \"sorting\" list created by parsing command-line options\n> (which is essentially used until the program exits). But we do so right\n> before leaving the cmd_ls_remote() function, which means we have to hit\n> all of the exits. But the point of UNLEAK() is that it's an annotation\n> which doesn't impact the variable itself. We can mark it as soon as\n> we're done writing its value, and then we only have to do so once.\n> \n> This gives us a minor code reduction, and serves as a better example of\n> how UNLEAK() can be used.\n\nLGTM. In hindsight, it's obvious that UNLEAK() can be called before we are\ndone using a variable. Otherwise, we'd just free() it instead.\n\n-Stolee\n\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n>  builtin/ls-remote.c | 8 +++-----\n>  1 file changed, 3 insertions(+), 5 deletions(-)\n> \n> diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c\n> index ea91679f33..092917eca2 100644\n> --- a/builtin/ls-remote.c\n> +++ b/builtin/ls-remote.c\n> @@ -83,6 +83,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n>  \t\t\t     PARSE_OPT_STOP_AT_NON_OPTION);\n>  \tdest = argv[0];\n>  \n> +\tUNLEAK(sorting);\n> +\n>  \tif (argc > 1) {\n>  \t\tint i;\n>  \t\tpattern = xcalloc(argc, sizeof(const char *));\n> @@ -107,7 +109,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n>  \n>  \tif (get_url) {\n>  \t\tprintf(\"%s\\n\", *remote->url);\n> -\t\tUNLEAK(sorting);\n>  \t\treturn 0;\n>  \t}\n>  \n> @@ -122,10 +123,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n>  \t\tint hash_algo = hash_algo_by_ptr(transport_get_hash_algo(transport));\n>  \t\trepo_set_hash_algo(the_repository, hash_algo);\n>  \t}\n> -\tif (transport_disconnect(transport)) {\n> -\t\tUNLEAK(sorting);\n> +\tif (transport_disconnect(transport))\n>  \t\treturn 1;\n> -\t}\n>  \n>  \tif (!dest && !quiet)\n>  \t\tfprintf(stderr, \"From %s\\n\", *remote->url);\n> @@ -150,7 +149,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n>  \t\tstatus = 0; /* we found something */\n>  \t}\n>  \n> -\tUNLEAK(sorting);\n>  \tref_array_clear(&ref_array);\n>  \treturn status;\n>  }\n> \n\n"},{"id":"403590","messageId":"CAPig+cTOcQymWWtSY3UN73_fpaWUs3u66+EZWBp1SvXeUrgsQQ@mail.gmail.com","threadId":"54036","inReplyTo":"20200813155426.GA896769@coredump.intra.peff.net","subject":"Re: [PATCH 0/2] UNLEAK style fixes","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-08-13T19:32:56Z","receivedAt":"2020-08-13T19:33:10Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Aug 13, 2020 at 11:54 AM Jeff King <peff@peff.net> wrote:\n> (As a side note, if we want to declare UNLEAK() a failure because nobody\n> cares enough to really use it, I'm OK with that, too).\n\nPerhaps the reason that UNLEAK() has not been particularly successful,\nin general, is that it requires extra knowledge and reasoning to know\nwhen to use it and how to do so properly. Couple that with the fact\nthat the scope of cases where it can be used is quite narrow compared\nto sum total of all code in project for which we simply free resources\nwhen we're done with them. So, it's hard to keep the specialized\nUNLEAK() knowledge in one's head.\n\nSpeaking from personal experience, the several times I have had to\ndeal with UNLEAK(), I had to re-learn it from scratch each time. That\nmeant studying the header comment, studying the implementation, and\nstudying existing callers before things \"clicked\" enough to be able to\nfeel confident about how to use it (assuming it wasn't false\nconfidence).\n\nEven today, reading this patch series, I had to go through all that\nagain just to understand the changes made by the patches, and\nespecially the commit message of patch [1/2]. It took several\nre-reads, plus re-examining UNLEAK() documentation, plus looking at\nthe UNLEAK() implementation a couple times before the [1/2] commit\nmessage finally \"clicked\".\n\nThat all represents a lot of cognitive overhead versus the common\npractice of simply freeing resources when you're done with them, which\nrequires no extra cognitive load since it is something we think about\n_always_ when working with a language like C with no built-in garbage\ncollection.\n\nSo, I for one would not be especially sad to see UNLEAK() retired.\n\n(The patch series itself looked fine and made sense once I had\nre-acquired the necessary UNLEAK() knowledge.)\n"},{"id":"403638","messageId":"20200814101706.GC3312240@coredump.intra.peff.net","threadId":"54036","inReplyTo":"275f3f6c-77ed-fddd-8f79-28f25b257362@gmail.com","subject":"Re: [PATCH 1/2] stop calling UNLEAK() before die()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-08-14T10:17:06Z","receivedAt":"2020-08-14T10:17:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Aug 13, 2020 at 02:08:45PM -0400, Derrick Stolee wrote:\n\n> On 8/13/2020 11:55 AM, Jeff King wrote:\n> > The point of UNLEAK() is to make a reference to a variable that is about\n> > to go out of scope so that leak-checkers will consider it to be\n> > not-leaked. Doing so right before die() is therefore pointless; even\n> > though we are about to exit the program, the variable will still be on\n> > the stack and accessible to leak-checkers.\n> > \n> > These annotations aren't really hurting anything, but they clutter the\n> > code and set a bad example of how to use UNLEAK().\n> \n> Good justification. I'll stop being a bad example ;)\n\nTo be fair, it seems clear that UNLEAK() as a concept is rather\nconfusing. I really never intended anybody to start sprinkling it around\nthe code. It was meant to be a tool for folks who are interested in\nrunning leak-checkers to do in-code annotations (for \"yes, I know this\nleaks but not until the program effectively ends\").\n\nI certainly don't mind if people writing new code preemptively annotate\nthis kind of leak. But I also wouldn't really encourage authors to put a\nlot of effort into it, given the current state of the annotations.\n\n-Peff\n"},{"id":"403639","messageId":"20200814103451.GD3312240@coredump.intra.peff.net","threadId":"54036","inReplyTo":"CAPig+cTOcQymWWtSY3UN73_fpaWUs3u66+EZWBp1SvXeUrgsQQ@mail.gmail.com","subject":"Re: [PATCH 0/2] UNLEAK style fixes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-08-14T10:34:51Z","receivedAt":"2020-08-14T10:34:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Aug 13, 2020 at 03:32:56PM -0400, Eric Sunshine wrote:\n\n> On Thu, Aug 13, 2020 at 11:54 AM Jeff King <peff@peff.net> wrote:\n> > (As a side note, if we want to declare UNLEAK() a failure because nobody\n> > cares enough to really use it, I'm OK with that, too).\n> \n> Perhaps the reason that UNLEAK() has not been particularly successful,\n> in general, is that it requires extra knowledge and reasoning to know\n> when to use it and how to do so properly. Couple that with the fact\n> that the scope of cases where it can be used is quite narrow compared\n> to sum total of all code in project for which we simply free resources\n> when we're done with them. So, it's hard to keep the specialized\n> UNLEAK() knowledge in one's head.\n> \n> Speaking from personal experience, the several times I have had to\n> deal with UNLEAK(), I had to re-learn it from scratch each time. That\n> meant studying the header comment, studying the implementation, and\n> studying existing callers before things \"clicked\" enough to be able to\n> feel confident about how to use it (assuming it wasn't false\n> confidence).\n\nI think this is really the meat of it. I never intended UNLEAK() to be\nsomething people dealt with unless they were trying to get LSAN or\nvalgrind to run without complaining.\n\n> That all represents a lot of cognitive overhead versus the common\n> practice of simply freeing resources when you're done with them, which\n> requires no extra cognitive load since it is something we think about\n> _always_ when working with a language like C with no built-in garbage\n> collection.\n\nTo be clear, I have no problem with _actually_ freeing resources if\nthat's an option. The point of UNLEAK() was:\n\n  - to help with structs that don't have an easy way to free all\n    elements (e.g., rev_info)\n\n  - to preempt arguments about whether calling free(buf) right before\n    programming exit is wasted effort. Whereas UNLEAK() is true\n    zero-cost for non-leak-checking builds.\n\n  - to avoid asking people to rewrite:\n\n      return foo(bar);\n\n     into:\n\n       ret = foo(bar);\n       free(bar);\n       return ret;\n\nSo we could go that direction, but I'd wait on it until somebody feels\nlike sinking some time into getting us leak-checker-clean.\n\nIn the meantime, I have a slight preference to leave UNLEAK() there as a\npotential tool for somebody digging into leak-checkers. But we almost\ncertainly shouldn't be asking new authors to use it in reviews, etc.\nTBH, I'm not sure why people starting sprinkling UNLEAK() around in the\nfirst place. ;)\n\n-Peff\n"},{"id":"403680","messageId":"CAPig+cTRPMSEG1gZPkjKDv2qUuFALvtr71-fiqWAiJkrkYZQ0g@mail.gmail.com","threadId":"54036","inReplyTo":"20200814103451.GD3312240@coredump.intra.peff.net","subject":"Re: [PATCH 0/2] UNLEAK style fixes","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-08-14T16:23:28Z","receivedAt":"2020-08-14T16:23:50Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Aug 14, 2020 at 6:35 AM Jeff King <peff@peff.net> wrote:\n> On Thu, Aug 13, 2020 at 03:32:56PM -0400, Eric Sunshine wrote:\n> > That all represents a lot of cognitive overhead versus the common\n> > practice of simply freeing resources when you're done with them, which\n> > requires no extra cognitive load since it is something we think about\n> > _always_ when working with a language like C with no built-in garbage\n> > collection.\n>\n> In the meantime, I have a slight preference to leave UNLEAK() there as a\n> potential tool for somebody digging into leak-checkers. But we almost\n> certainly shouldn't be asking new authors to use it in reviews, etc.\n\nI don't think it works that way in practice, though. There are enough\nUNLEAK()'s sprinkled around that anyone working on or around code with\nan existing UNLEAK() is compelled to understand/[re-]study it in order\nto avoid breaking existing uses and/or to correctly mirror existing\nuses when dealing with new resource allocations.\n\nThe same applies to patches. As a reviewer, I have two choices when I\nsee UNLEAK(): either I ignore it because I don't have the specialized\nknowledge in my head (which makes me feel like my review is\nineffective), or I re-acquire the knowledge. And it's not just patches\nlike the ones in this series which are actively adjusting UNLEAK()\ncallers, but any patch which adds or removes an UNLEAK() corresponding\nto the central meaty changes of the patch, or even a patch in which\nUNLEAK() appears only in context lines, or even patches which don't\ncontains any UNLEAK() calls, but the source file to which the patch\napplies does use UNLEAK(), if the reviewer consults the original\nsource code in addition to the patch.\n\n> TBH, I'm not sure why people starting sprinkling UNLEAK() around in the\n> first place. ;)\n\nFor the same reason that people are concerned about calling free() or\notherwise releasing or unlocking resources which they have acquired:\nthey're trying to be responsible. When a programmer sees UNLEAK()\nbeing used in or around the code being changed, he or she will attempt\nto maintain the fidelity of the existing code by being careful to\nmimic existing nearby resource handling practices.\n"}]}