[webkit-reviews] review granted: [Bug 68554] IndexedDB: compare strings without decoding : [Attachment 108231] Patch

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has granted Joshua Bell <jsbell at chromium.org>'s
request for review:
Bug 68554: IndexedDB: compare strings without decoding
https://bugs.webkit.org/show_bug.cgi?id=68554

Attachment 108231: Patch
https://bugs.webkit.org/attachment.cgi?id=108231&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=108231&action=review


> Source/WebCore/storage/IDBLevelDBCoding.cpp:303
> +int compareEncodedStringsWithLength(const char* p, const char* limitP, const
char* q, const char* limitQ)

I find these 1 letter variable names hard to read, but it looks like that's how
all the code around it is.  It would be great to have a follow up patch that
uses more descriptive variable names in this file.

> Source/WebCore/storage/IDBLevelDBCoding.cpp:314
> +    const unsigned lmin = lenP < lenQ ? lenP : lenQ;

Nit: Should lmin be a size_t since that's what memcmp takes?  Also, it might be
more readable to explicitly static_cast<size_t>() after asserting that lenP and
lenQ are >= 0.