Christ van Willegen wrote:
> On Tue, Aug 25, 2015 at 4:31 PM, Bram Moolenaar <[email protected]> wrote:
>> ***************
>> *** 5068,5086 ****
>> if (s1 == NULL || s2 == NULL)
>> return FALSE;
>>
>> ! if (STRLEN(s1) != STRLEN(s2))
>> ! return FAIL;
>> !
>> ! for (i = 0; s1[i] != NUL && s2[i] != NUL; i += MB_PTR2LEN(s1 + i))
>> {
>> int c1 = PTR2CHAR(s1 + i);
>> ! int c2 = PTR2CHAR(s2 + i);
>>
>> if ((p_fic ? MB_TOLOWER(c1) != MB_TOLOWER(c2) : c1 != c2)
>> && (prev1 != '*' || prev2 != '*'))
>> return FAIL;
>> prev2 = prev1;
>> prev1 = c1;
>> }
>> return TRUE;
>> }
>> --- 5068,5086 ----
>> if (s1 == NULL || s2 == NULL)
>> return FALSE;
>>
>> ! for (i = 0, j = 0; s1[i] != NUL;)
>> {
>> int c1 = PTR2CHAR(s1 + i);
>> ! int c2 = PTR2CHAR(s2 + j);
>>
>> if ((p_fic ? MB_TOLOWER(c1) != MB_TOLOWER(c2) : c1 != c2)
>> && (prev1 != '*' || prev2 != '*'))
>> return FAIL;
>> prev2 = prev1;
>> prev1 = c1;
>> +
>> + i += MB_PTR2LEN(s1 + i);
>> + j += MB_PTR2LEN(s2 + j);
>> }
>> return TRUE;
>> }
>
> Was the test for s2[j] != NUL left out intentionally, or does another
> code path catch that one?
I assume that you're talking about line misc2.c:5071:
!5071 for (i = 0, j = 0; s1[i] != NUL;)
5072 {
5073 int c1 = PTR2CHAR(s1 + i);
5074 int c2 = PTR2CHAR(s2 + j);
5075
5076 if ((p_fic ? MB_TOLOWER(c1) != MB_TOLOWER(c2) : c1 != c2)
5077 && (prev1 != '*' || prev2 != '*'))
5078 return FAIL;
5079 prev2 = prev1;
5080 prev1 = c1;
5081
5082 i += MB_PTR2LEN(s1 + i);
5083 j += MB_PTR2LEN(s2 + j);
5084 }
5085 return TRUE;
At first I thought that testing for s2[j] != NUL was useless
at line 5071, since if s2[j] is NUL, then the test at line 5076
would be false and so function would return at line 5078.
But I now see 2 reasons why that may not be true:
- if s2 ends with "**" then (prev1 != '*' || prev2 != '*')
at line 5077 will be false and the loop will access beyond
of string for s2! (bug!)
- or if the is s1[i] contains an invalid utf8 sequence
such as: 0xc0 0x80 for which PTR2CHAR(...) is 0.
and s2[j] is NUL, then c1 and c2 will be equal and
the loop will continue, hence also accessing beyond
end of string s2 (bug!).
So it's buggy :-(
It's also odd that function returns TRUE, FALSE or FAIL.
That was not introduced by patch 7.4.835.
The return FAIL should be return FALSE at line 5078.
How about following patch?
Regards
Dominique
--
--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php
---
You received this message because you are subscribed to the Google Groups
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
diff --git a/src/misc2.c b/src/misc2.c
index a4a65d6..379916b 100644
--- a/src/misc2.c
+++ b/src/misc2.c
@@ -5059,6 +5059,8 @@ ff_wc_equal(s1, s2)
char_u *s2;
{
int i, j;
+ int c1 = NUL;
+ int c2 = NUL;
int prev1 = NUL;
int prev2 = NUL;
@@ -5068,21 +5070,21 @@ ff_wc_equal(s1, s2)
if (s1 == NULL || s2 == NULL)
return FALSE;
- for (i = 0, j = 0; s1[i] != NUL;)
+ for (i = 0, j = 0; s1[i] != NUL && s2[j] != NUL;)
{
- int c1 = PTR2CHAR(s1 + i);
- int c2 = PTR2CHAR(s2 + j);
+ c1 = PTR2CHAR(s1 + i);
+ c2 = PTR2CHAR(s2 + j);
if ((p_fic ? MB_TOLOWER(c1) != MB_TOLOWER(c2) : c1 != c2)
&& (prev1 != '*' || prev2 != '*'))
- return FAIL;
+ return FALSE;
prev2 = prev1;
prev1 = c1;
i += MB_PTR2LEN(s1 + i);
j += MB_PTR2LEN(s2 + j);
}
- return TRUE;
+ return c1 == c2;
}
#endif
