Re: [PATCH/RFC v2 1/2] Strip namelen out of ce_flags into a ce_namelen field
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:54:14
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2016-06-15 · Re: [PATCH/RFC v2 1/2] Strip namelen out of ce_flags into a ce_namelen field · Junio C Hamano <hidden>
- 2016-06-15 · [PATCH/RFC v2 1/2] Strip namelen out of ce_flags into a ce_namelen field · Thomas Gummerer <hidden>
Junio C Hamano [off-list ref] writes:
quoted
@@ -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 <redacted> --- 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