Re: [PATCH/RFC v2 1/2] Strip namelen out of ce_flags into a ce_namelen field
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 11, 2012, 16:29 UTC
- Message-ID
- <7vsjcyfehg.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <7vy5mtlebz.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
Show 18 quoted lines
>> @@ -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 <gitster@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