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

[bugzilla-daemon at webkit.org]

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





--- Comment #8 from Fumitoshi Ukai <ukai at chromium.org>  2009-09-02 18:29:24 PDT ---
(In reply to comment #6)
> (From update of attachment 38917 [details])
> 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.

Thanks for review. Fixed those things.

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