Re: [PATCH] SBuf c-string comparisons

On 15/04/2014 7:35 a.m., Alex Rousskov wrote:
> On 04/14/2014 11:07 AM, Amos Jeffries wrote:
>> On 15/04/2014 2:56 a.m., Alex Rousskov wrote:
>>> On 04/14/2014 04:49 AM, Amos Jeffries wrote:
>>>> I've added a unit test to catch the rare \0 mid-string case I spoke of:
>>>> * SBuf("foo\0test").compare("foo", 9);
>>>>
>>>> It works fine up to the point N(4) > strlen("foo"). After that point our
>>>> function returns 1 to indicate that the SBuf is a longer string, whereas
>>>> strncmp/strncasecmp return 0 up to infinity.
>>>
>>> Yes, this is related to the large-n handling bug I keep talking about.
>>> IMO, this must be fixed as previously discussed: C-string API should not
>>> look past the first null character.
>>>
>>>
>>>> The code as presented earlier copes with the weirdness fine.
>>>
>>>
>>> AFAICT, the latest posted patch accesses non-existent c-string bytes
>>> under certain conditions (large n, large SBuf with trailing null
>>> characters, short s matching the c-string in SBuf). Do you agree? If you
>>> do agree, please post a fixed patch. If not, I would have to spend time
>>> writing a test case to prove my point against the last patch posted.
>>
>> I agree.
>
> Great.
>
>
>> Attached patch implements what we agreed on in IRC.
>
> Please note that we discussed only one aspect of the code; there was no
> blueprint on how to get everything working, and there are many ways to
> do that.
>
>
>> It produces wrong return value in two case states. Marked with "BUG 1"
>> and "BUG 2" in the patch.
>
>
>> + // BUG 1: when length() < n - buffer overruns on buf().
>
> byteCompareLen cannot exceed length() so there should not be buf()
> overruns. However, there are other bugs as discussed below.
>
>
>> + // BUG 2: when length() == strlen(s) < n, no terminator to match against in buf()
>
>
> Yes, the lack of buf() terminator still needs to be fixed for all cases
> where the terminator would have been used by a c-string-based
> implementation. Please see the IRC log for some specific suggestions.

The attached patch passes all the tests including \0 embeded in the strings.
 It also avoids the s[] access by using strlen(s) != byteCompareLen.

Amos