[llvm-commits] [llvm] r50821 - /llvm/trunk/include/llvm/ADT/StringExtras.h

[Ted Kremenek]

On May 7, 2008, at 11:45 AM, Bill Wendling wrote:

> On Wed, May 7, 2008 at 11:35 AM, Ted Kremenek <kremenek at apple.com>  
> wrote:
>> +/// CStrInCStrNoCase - Portable version of strcasestr.  Locates  
>> the first
>> +///  occurance of c-string 's1' in string 's2', ignoring case.   
>> Returns
>> +///  NULL if 's1' cannot be found.
>> +static inline const char* CStrInCStrNoCase(const char *s1, const  
>> char *s2) {
>> +
>> +  // Are either strings NULL?
>> +  if (!s1 || !s2)
>> +    return 0;
>> +
>
> Should there be an early return for when s1 and s2 are the same value?

Yes.  Good idea.
>>

>> +  const char *I1=s1, *I2=s2;
>> +
>> +  while (*I1 != '\0' || *I2 != '\0' )
>> +    if (tolower(*I1) != tolower(*I2)) { // No match.  Start over.
>> +      ++s1; I1 = s1; I2 = s2;
>
> If you increment s1 here, then the function's comment is wrong. Do you
> mean to increment s2 instead?

Good catch.  I was thinking of strcasestr, which looks for s2 in s1,  
not s1 in s2.  I need to fix this by incrementing s2 instead.

>> +    }
>> +    else { // Character match.  Advance to the next character.
>> +      ++I1; ++I2;
>> +    }
>> +
>> +  // If we exhausted all of the characters in 's2', then 's1' does  
>> not occur
>> +  // in it.
>> +  return *I2 == '\0' ? 0 : I1;
>
> Is this correct? If we have something like:
>
> s1 = "bar"
> s2 = "foobar"
>
> then I2 will be \0 here, but we still found s1 in there.

Yeah this looks completely wrong.  What about (assuming we increment  
s2 instead of s1):

   return *I1 == '\0' ? s2 : 0;