[Webkit-unassigned] [Bug 28901] strnstr in wtf/StringExtras.h

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=28901


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #38917|review?                     |review+
               Flag|                            |




--- Comment #6 from Darin Adler <darin at apple.com>  2009-09-02 09:18:55 PDT ---
(From update of attachment 38917)
This is a fairly long function. Do we really want it as an inline? I suppose we
don't have a StringExtras.cpp so this is OK for now.

The name "find" is not really good as a name for the second argument, because
it's a verb, not a noun. The second argument is the string to find, not "a
find". So I suggest calling it something like "target".

> +    int findLength = strlen(find);

The result of strlen is a size_t, not an int. There's no reason to use int
instead of size_t here.

> +    for (const char* start = buffer; *start && start + findLength <= buffer + bufferLength; start++) {
> +        if (*start == *find && strncmp(start, find, findLength) == 0)
> +            return const_cast<char*>(start);
> +    }

I'd think you'd want to call strncmp on start + 1 and find + 1 and findLength -
1 instead.

r=me but I think it's worth fixing some of those things I mentioned. So you
could clear the review flag if you intend to post a new patch.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.