Re: [PATCH] SBuf c-string comparisons

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.
Received on Tue Apr 15 2014 - 17:07:52 MDT