On 04/15/2014 08:55 AM, Amos Jeffries wrote:
> The attached patch passes all the tests including \0 embeded in the strings.
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.
Please add the following test case, just in case (it does not expose any
bugs right now AFAICT):
testComparisonStdOneWay("", "");
Please note that I get a warning when compiling your patch on a 64 bit
OS because our MemBlob/SBuf::size_type is smaller than size_t:
> tests/testSBuf.cc: In member function ‘void testSBuf::testComparisons()’:
> tests/testSBuf.cc:360: error: no matching function for call to ‘min(uint32_t,
> size_t)’
Did we consciously made MemBlob/SBuf::size_type 32 bit for some reason
or was it an accident?
HTH,
Alex.
