Re: [PATCH v2 3/5] sha1_file: improve directories comparison method

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 6, 2011, 16:32 UTC
Message-ID: <7vliu1aay2.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1315304645-12009-4-git-send-email-Hui.Wang@windriver.com>

Wang Hui <Hui.Wang@windriver.com> writes:

Show 12 quoted lines

> From: Hui Wang <Hui.Wang@windriver.com>
>
> In the past, to check if two directory paths are same, we use memcmp()
> to directly compare their path strings, this method can't get an
> accurate result if paths include ".." or "." or redundant slash, e.g.
> current dir is /, "/a/b/c", "/a/b//c/d/e/../.." and "./a/b/f/../c"
> should be the same dir, but current method will identify they are
> different.
>
> Now add a global function is_same_directory() to replace the old
> memcmp() method, this function will change two input paths to real
> path first, then normalized them and compare them.

I do not like this patch _at all_. While it may result in correct result if you _always_ make it absolute before comparing two entities, if you will be storing the normalized result after running the comparison anyway, and if you are comparing against the existing and supposedly already normalized entities with a new candidate, why would anybody sane would want to keep paying for the normalization cost at such a low level?

IOW, you are proposing to do:

	given a new candidate;
	for existing entities:
		normalize existing
                normalize candiate
                compare the above two
                if they are equal:
                	ignore
	if no match found
        	add the normalized candidate to the list

Wouldn't it make much more sense to do this:

	given a new candidate;
        normalize it
	for existing entities:
                compare existing and normalized candidate
		there is no point in normalizing the existing one!
                if they are equal:
                	ignore
	if no match found
        	add the normalized candidate to the list