git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Michael Haggerty" <mhagger@alum.mit.edu>,
	git@vger.kernel.org, "Henrik Grubbström" <grubba@grubba.org>,
	git-dev@github.com
Subject: [PATCH] attr: don't confuse prefixes with leading directories
Date: Tue, 10 Jan 2012 13:08:21 -0500	[thread overview]
Message-ID: <20120110180820.GA15273@sigill.intra.peff.net> (raw)
In-Reply-To: <20120110171100.GA18962@sigill.intra.peff.net>

When we prepare the attribute stack for a lookup on a path,
we start with the cached stack from the previous lookup
(because it is common to do several lookups in the same
directory hierarchy). So the first thing we must do in
preparing the stack is to pop any entries that point to
directories we are no longer interested in.

For example, if our stack contains gitattributes for:

  foo/bar/baz
  foo/bar
  foo

but we want to do a lookup in "foo/bar/bleep", then we want
to pop the top element, but retain the others.

To do this we walk down the stack from the top, popping
elements that do not match our lookup directory. However,
the test do this simply checked strncmp, meaning we would
mistake "foo/bar/baz" as a leading directory of
"foo/bar/baz_plus". We must also check that the character
after our match is '/', meaning we matched the whole path
component.

There are two special cases to consider:

  1. The top of our attr stack has the empty path. So we
     must not check for '/', but rather special-case the
     empty path, which always matches.

  2. Typically when matching paths in this way, you would
     also need to check for a full string match (i.e., the
     character after is '\0'). We don't need to do so in
     this case, though, because our path string is actually
     just the directory component of the path to a file
     (i.e., we know that it terminates with "/", because the
     filename comes after that).

Helped-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Jeff King <peff@peff.net>
---
I wrote it in the minimally-intrusive way, but you could also pull
the "!namelen" case before the strncmp (since a strncmp of size 0 is
always true, anyway). I don't know if it would be more obvious that way.

I prepared this on top of master, but the patch applies (with some
shifted line counts) on older releases, too.

 attr.c                |    3 ++-
 t/t0003-attributes.sh |   10 ++++++++++
 2 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/attr.c b/attr.c
index 76b079f..fa975da 100644
--- a/attr.c
+++ b/attr.c
@@ -582,7 +582,8 @@ static void prepare_attr_stack(const char *path)
 
 		elem = attr_stack;
 		if (namelen <= dirlen &&
-		    !strncmp(elem->origin, path, namelen))
+		    !strncmp(elem->origin, path, namelen) &&
+		    (!namelen || path[namelen] == '/'))
 			break;
 
 		debug_pop(elem);
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index dbb2623..51f3045 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -159,6 +159,16 @@ test_expect_success 'relative paths' '
 	(cd b && attr_check ../a/b/g a/b/g)
 '
 
+test_expect_success 'prefixes are not confused with leading directories' '
+	attr_check a_plus/g unspecified &&
+	cat >expect <<-\EOF &&
+	a/g: test: a/g
+	a_plus/g: test: unspecified
+	EOF
+	git check-attr test a/g a_plus/g >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'core.attributesfile' '
 	attr_check global unspecified &&
 	git config core.attributesfile "$HOME/global-gitattributes" &&
-- 
1.7.9.rc0.33.gd3c17

  reply	other threads:[~2012-01-10 18:08 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-10  7:03 [BUG] gitattribute macro expansion oddity Jeff King
2012-01-10  9:01 ` Michael Haggerty
2012-01-10 17:11   ` Jeff King
2012-01-10 18:08     ` Jeff King [this message]
2012-01-10 18:21       ` [PATCH] attr: don't confuse prefixes with leading directories Jeff King
2012-01-10 19:23         ` Junio C Hamano
2012-01-10 19:28           ` Jeff King
2012-01-10 19:32             ` Jeff King
2012-01-10 20:25             ` Junio C Hamano
2012-01-10 22:31               ` Jeff King
2012-01-10 19:25       ` Junio C Hamano
2012-01-10 17:22 ` [BUG] gitattribute macro expansion oddity Junio C Hamano
2012-01-11  4:37   ` Michael Haggerty

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=20120110180820.GA15273@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git-dev@github.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=grubba@grubba.org \
    --cc=mhagger@alum.mit.edu \
    /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).