Re: [PATCH] SBuf c-string comparisons

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.
Received on Wed Apr 16 2014 - 14:32:49 MDT