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

["Shawn O. Pearce" <spearce@spearce.org>]

Junio C Hamano <gitster@pobox.com> wrote:
> +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.