Re: [PATCH v2 08/51] is_dup_ref(): extract function from sort_ref_array()

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 12, 2011, 22:33 UTC
Message-ID: <7vvcplxq4r.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1323668338-1764-9-git-send-email-mhagger@alum.mit.edu>

mhagger@alum.mit.edu writes:

Show 18 quoted lines

> +/*
> + * Emit a warning and return true iff ref1 and ref2 have the same name
> + * and the same sha1.  Die if they have the same name but different
> + * sha1s.
> + */
> +static int is_dup_ref(const struct ref_entry *ref1, const struct ref_entry *ref2)
> +{
> +	if (!strcmp(ref1->name, ref2->name)) {
> +		/* Duplicate name; make sure that the SHA1s match: */
> +		if (hashcmp(ref1->sha1, ref2->sha1))
> +			die("Duplicated ref, and SHA1s don't match: %s",
> +			    ref1->name);
> +		warning("Duplicated ref: %s", ref1->name);
> +		return 1;
> +	} else {
> +		return 0;
> +	}
> +}

At this step, is_dup_ref() is only called from sort_ref_array() which in turn is only called on either a collection of loose or packed refs, so warning is warranted. IOW I do not see anything wrong with _this_ patch.

Later in the series, however, the collection of extra refs seems to be shoved into a phoney "direntry" and made to go through the same add_ref() machinery that uses find_containing_direntry() which in turn calls search_ref_dir() that smells like a definite no-no.