Re: [PATCH 8/8] git-gc --auto: run "repack -A -d -l" as necessary.

From: Shawn O. Pearce <spearce@spearce.org>
Date: Sep 18, 2007, 02:59 UTC
Message-ID: <20070918025926.GI3099@spearce.org>
In-Reply-To: <1190018716666-git-send-email-gitster@pobox.com>

Junio C Hamano <gitster@pobox.com> wrote:

Show 19 quoted lines

> +static int too_many_packs(void)
> +{
> +	struct packed_git *p;
> +	int cnt;
> +
> +	if (gc_auto_pack_limit <= 0)
> +		return 0;
> +
> +	for (cnt = 0, p = packed_git; p; p = p->next) {
> +		char *suffix;
> +		int keep;
> +		if (!p->pack_local)
> +			continue;
> +		suffix = p->pack_name + strlen(p->pack_name) - 5;
> +		if (memcmp(suffix, ".pack", 6))
> +			continue;
> +		memcpy(suffix, ".keep", 6);
> +		keep = access(p->pack_name, F_OK) && (errno == ENOENT);
> +		memcpy(suffix, ".pack", 6);

*rubs eyes* Am I reading that right? You are modifying pack_name in the middle of this function? WHY is pack_name not const char*? *sigh*. I'd NEVER consider doing that. Not in a million years. Because someday someone is going to make a change that's going to break that somehow, like by jumping out of this loop early and not fixing pack_name up first.

Yea, sure, there's no way today that this would fail. But I like to think that some poor sap is going to have to read my code after I write it and that they should not be forced to deal with stuff like this when such a time comes.

-- 
Shawn.