From: Junio C Hamano <gitster@pobox.com>
To: Wang Hui <Hui.Wang@windriver.com>
Cc: <git@vger.kernel.org>, <tali@admingilde.org>
Subject: Re: [PATCH v2 3/5] sha1_file: improve directories comparison method
Date: Tue, 06 Sep 2011 09:32:21 -0700 [thread overview]
Message-ID: <7vliu1aay2.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <1315304645-12009-4-git-send-email-Hui.Wang@windriver.com> (Wang Hui's message of "Tue, 6 Sep 2011 18:24:03 +0800")
Wang Hui <Hui.Wang@windriver.com> writes:
> 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
next prev parent reply other threads:[~2011-09-06 16:32 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-09-06 10:24 [PATCH v2 0/5] sha1_file: remove only current repository can have relative path limitation Wang Hui
2011-09-06 10:24 ` [PATCH v2 1/5] sha1_file cleanup: remove redundant variable check Wang Hui
2011-09-06 10:24 ` [PATCH v2 2/5] sha1_file: remove a buggy value setting Wang Hui
2011-09-06 10:24 ` [PATCH v2 3/5] sha1_file: improve directories comparison method Wang Hui
2011-09-06 10:24 ` [PATCH v2 4/5] sha1_file: remove relative entries limitation Wang Hui
2011-09-06 10:24 ` [PATCH v2 5/5] t5710: add testcase for multi-level relative alternates Wang Hui
2011-09-06 16:32 ` Junio C Hamano [this message]
2011-09-06 16:26 ` [PATCH v2 2/5] sha1_file: remove a buggy value setting Junio C Hamano
2011-09-07 9:55 ` wanghui
2011-09-06 16:59 ` [PATCH v2 1/5] sha1_file cleanup: remove redundant variable check Junio C Hamano
2011-09-07 10:24 ` wanghui
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7vliu1aay2.fsf@alter.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=Hui.Wang@windriver.com \
--cc=git@vger.kernel.org \
--cc=tali@admingilde.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).