On Thu, Mar 14, 2019 at 12:54:37AM +0100, Ævar Arnfjörð Bjarmason wrote:
> Change an idiom we're using to ensure that gc_before_repack() only
> does work once (see 62aad1849f ("gc --auto: do not lock refs in the
> background", 2014-05-25)) to be more obvious.
>
> Nothing except this function cares about the "pack_refs" and
> "prune_reflogs" variables, so let's not leave the reader wondering if
> they're being zero'd out for later use somewhere else.
I agree the existing code is not very obvious about what it's trying to
do. I think a comment would have helped a lot.
Your rewrite is definitely better than the original, but I think it
might also benefit from a comment. I.e., something like:
> static void gc_before_repack(void)
> {
/*
* We may be called twice, as both the pre- and post-daemonized
* phases will call us, but running these commands more than
* once is pointless and wasteful.
*/
> + static int done = 0;
> + if (done++)
> + return;
-Peff
