Re: [PATCH v12 4/4] ls-remote: create '--sort' option

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 9, 2018, 00:48 UTC
Message-ID: <xmqqmuyd6w7d.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <CAHwyqnWe=KmmdGJJ_DnCK+D_FoC0sAQG_8=_p+4+Yxmr8s8sPA@mail.gmail.com>

Harald Nordgren <haraldnordgren@gmail.com> writes:

Show 10 quoted lines

>> It is not too big a deal, but I find that liberal sprinkling of
>> UNLEAK() is somewhat unsightly.  From the code we leave with this
>> change, it is clear what we'd need to do when we make the code fully
>> leak-proof (i.e. we'd have a several lines to free resources held by
>> these two variables here, and then UNLEAK() that appear earlier in
>> the function will become a "goto cleanup;" after setting appropriate
>> value to "status"), so let's not worry about it.
>
> Removed the "extra" unleak annotations within the if-return blocks,
> but kept them at the end.

I actually think that is worse. The UNLEAK()s near early-return serve to remind us: "here we are making early return and when we properly plug the leak, here is one of the places we need to fix". Please keep them, unless (and I do not recommend to do this) you are plugging the leaks for real.