Re: StringNG merge?

On 12/16/2012 03:02 PM, Kinkie wrote:
>>> >> + if (buf()+length() < tmp+str.length()) { //not enough chars to match the whole needle
>>> >> + debugs(SBUF_DEBUGSECTION,8,HERE << "needle too big for haystack");
>>> >> + end=tmp-1;
>>> >> + continue;
>>> >> + }
>> >
>> > If I interpret STL documentation correctly, the limit is not buf() +
>> > length() but endpos. We should not match against characters beyond
>> > endpos, even if the match _starts_ before endpos. Please double check me
>> > on that.

> I am not 100% sure I understand what you mean, but this simple test:

> std::string haystack("foo bar gazonk");
> std:;string needle("gazonka");
> cout << "find overflow: " << haystack.find(needle) << endl;
>
> returns npos (as I'd expect)

What I meant is that

    "123".rfind("23", 2)

should return npos instead of 1 because "23" is not found in the first
two characters of the haystack ("123") string. So the above if-statement
condition should use buf+endpos rather than buf+length().

Does that clarify?

The code has been rewritten since then, but it is still buggy:

> char *bufBegin = buf();
> char *cur = bufBegin+length()-needle.length();

The "cur" variable should start with bufBegin+endPos-needle.length() or
equivalent.

BTW, this implies that you do not have a basic "match beyond head" unit
test case:

  "headtail".rfind("t", 4) == npos

and I would also add

    "headmiddletail".rfind("middle", 9) == npos
    "headmiddletail".rfind("middle", 10) == 4

(at least).

And, for every manually added haystack.*find(needle, n) test case, I
would automatically add a haystack.substr(0, n).*find(needle) test case,
since both must return the same value for every haystack, needle, and n
(AFAICT).

Again, all of the above assumes that my interpretation of
std::string::rfind() API is correct. That is what we are mimicking here.

Thank you,

Alex.
Received on Sat Jan 12 2013 - 19:32:46 MST