Junio C Hamano <gits...@pobox.com> writes:

>> @@ -395,10 +395,8 @@ int df_name_compare(const char *name1, int len1, int 
>> mode1,
>>      return c1 - c2;
>>  }
>>  
>> -int cache_name_compare(const char *name1, int flags1, const char *name2, 
>> int flags2)
>> +int cache_name_stage_compare(const char *name1, int stage1, int len1, const 
>> char *name2, int stage2, int len2)
>>  {
>> -    int len1 = flags1 & CE_NAMEMASK;
>> -    int len2 = flags2 & CE_NAMEMASK;
>>      int len = len1 < len2 ? len1 : len2;
>>      int cmp;
>
> Isn't this a _BUGFIX_?  It appears to me that the original code
> would only compare the first 4k bytes and ignore the rest, if two
> cache entries, both with overlong names, are compared.  Care to come
> up with a test case to demonstrate the breakage and a bugfix without
> the remainder of this patch, to be applied to 'master' and older
> maintenance releases?

Perhaps something like this (based on v1.7.0.9 as this may deserve
to go to older maintenance releases).

-- >8 --
Subject: [PATCH] cache_name_compare(): do not truncate while comparing paths

We failed to use ce_namelen() equivalent and instead only compared
up to the CE_NAMEMASK bytes by mistake.  Adding an overlong path
that shares the same common prefix as an existing entry in the index
did not add a new entry, but instead replaced the existing one, as
the result.

Signed-off-by: Junio C Hamano <gits...@pobox.com>
---
 read-cache.c             | 13 +++++++++----
 t/t3006-ls-files-long.sh | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 4 deletions(-)
 create mode 100755 t/t3006-ls-files-long.sh

diff --git a/read-cache.c b/read-cache.c
index f1f789b..0cd13aa 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -405,10 +405,15 @@ int df_name_compare(const char *name1, int len1, int 
mode1,
 
 int cache_name_compare(const char *name1, int flags1, const char *name2, int 
flags2)
 {
-       int len1 = flags1 & CE_NAMEMASK;
-       int len2 = flags2 & CE_NAMEMASK;
-       int len = len1 < len2 ? len1 : len2;
-       int cmp;
+       int len1, len2, len, cmp;
+
+       len1 = flags1 & CE_NAMEMASK;
+       if (CE_NAMEMASK <= len1)
+               len1 = strlen(name1 + CE_NAMEMASK) + CE_NAMEMASK;
+       len2 = flags2 & CE_NAMEMASK;
+       if (CE_NAMEMASK <= len2)
+               len2 = strlen(name2 + CE_NAMEMASK) + CE_NAMEMASK;
+       len = len1 < len2 ? len1 : len2;
 
        cmp = memcmp(name1, name2, len);
        if (cmp)
diff --git a/t/t3006-ls-files-long.sh b/t/t3006-ls-files-long.sh
new file mode 100755
index 0000000..202ad65
--- /dev/null
+++ b/t/t3006-ls-files-long.sh
@@ -0,0 +1,39 @@
+#!/bin/sh
+
+test_description='overly long paths'
+. ./test-lib.sh
+
+test_expect_success setup '
+       p=filefilefilefilefilefilefilefile &&
+       p=$p$p$p$p$p$p$p$p$p$p$p$p$p$p$p$p &&
+       p=$p$p$p$p$p$p$p$p$p$p$p$p$p$p$p$p &&
+
+       path_a=${p}_a &&
+       path_z=${p}_z &&
+
+       blob_a=$(echo frotz | git hash-object -w --stdin) &&
+       blob_z=$(echo nitfol | git hash-object -w --stdin) &&
+
+       pat="100644 %s 0\t%s\n"
+'
+
+test_expect_success 'overly-long path by itself is not a problem' '
+       printf "$pat" "$blob_a" "$path_a" |
+       git update-index --add --index-info &&
+       echo "$path_a" >expect &&
+       git ls-files >actual &&
+       test_cmp expect actual
+'
+
+test_expect_success 'overly-long path does not replace another by mistake' '
+       printf "$pat" "$blob_a" "$path_a" "$blob_z" "$path_z" |
+       git update-index --add --index-info &&
+       (
+               echo "$path_a"
+               echo "$path_z"
+       ) >expect &&
+       git ls-files >actual &&
+       test_cmp expect actual
+'
+
+test_done
-- 
1.7.11
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to