Summary: | Add WTFString method to compare equality with Vector<UChar> | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adam Barth <abarth> | ||||
Component: | New Bugs | Assignee: | Adam Barth <abarth> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | abarth, darin, dglazkov, eric, webkit.review.bot | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Other | ||||||
OS: | OS X 10.5 | ||||||
Bug Depends on: | |||||||
Bug Blocks: | 49845 | ||||||
Attachments: |
|
Description
Adam Barth
2011-01-27 16:17:26 PST
Created attachment 80373 [details]
Patch
Comment on attachment 80373 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=80373&action=review Seems OK. > Source/JavaScriptCore/wtf/text/StringImpl.h:366 > + if (!a.size()) > + return true; > + return !memcmp(a.data(), b->characters(), b->length()); Is calling memcmp with length = 0 OK? If so then you don't need the !a.size() check. Comment on attachment 80373 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=80373&action=review >> Source/JavaScriptCore/wtf/text/StringImpl.h:366 >> + if (!a.size()) >> + return true; >> + return !memcmp(a.data(), b->characters(), b->length()); > > Is calling memcmp with length = 0 OK? If so then you don't need the !a.size() check. Sounds dodgy: http://coding.derkeiler.com/Archive/C_CPP/comp.lang.c/2004-07/0109.html (In reply to comment #3) > > Is calling memcmp with length = 0 OK? If so then you don't need the !a.size() check. > > Sounds dodgy: > http://coding.derkeiler.com/Archive/C_CPP/comp.lang.c/2004-07/0109.html Why the FUD? memcpy with length == 0 is definitely OK! (In reply to comment #4) > Why the FUD? memcpy with length == 0 is definitely OK! I mean memcmp. > Why the FUD? memcpy with length == 0 is definitely OK!
Even if one of the pointers is NULL?
(In reply to comment #6) > > Why the FUD? memcmp with length == 0 is definitely OK! > > Even if one of the pointers is NULL? In practice, yes. I am pretty damned sure of this. And I think we rely on it. But the C standard does use wording that makes it seem a little ambiguous. Okiedokes. Thanks. Committed r76894: <http://trac.webkit.org/changeset/76894> Attachment 80373 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7597383 http://trac.webkit.org/changeset/76894 might have broken GTK Linux 64-bit Debug |