On 04/16/2014 12:05 AM, Amos Jeffries wrote: > On 16/04/2014 5:07 a.m., Alex Rousskov wrote: >> If I am reading the code correctly, there is a new bug: >> >>> It also avoids the s[] access by using strlen(s) != byteCompareLen. >> >>> + if (byteCompareLen < n && strlen(s) != byteCompareLen) >> >> s is guaranteed to be 0-terminated when n == npos only. For other cases, >> we do not have such a guarantee and, hence, cannot use strlen(). Using >> strlen(s) when n is not npos may lead to s overreads.
> I don't see any way around this without hand-crafing a full byte-by-byte > strncmp replacement. I am not against hand-crafting if it is really necessary -- we already hand-craft memCaseCmp IIRC. Personally, I would hand-craft _if_ system implementation of strncmp() is just a basic loop rather than some complicated, optimized low-level code. Otherwise, I would find a way to avoid strlen(). The following cloning approach also works, but I hope we do not have to pay such a high performance price here: SBuf clone(*this); return ... ? strncmp(clone.c_str(), s, n) : strncasecmp(...); > Which would appear to be overkill for this > transitional method (only needed until all I/O and globals/constants are > SBuf based). Since the hand-crafted implementation is simple, I do not consider it an overkill. And I am sure there is a way to avoid it if needed. > I will comment the new methods with "Requires S to be null-terminated.". I do not see why we should change (and limit) the "standard" API in this case. > Is this acceptible to you after those minor changes? I disagree that we should limit the API to require terminated c-strings. Sorry, Alex.
