Web lists-archives.com

Re: [PATCH 3/5] gc: refactor a "call me once" pattern




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