[webkit-reviews] review granted: [Bug 60637] We should have a Content-Type parser : [Attachment 93151] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed May 11 11:54:25 PDT 2011


Adam Barth <abarth at webkit.org> has granted Jay Civelli
<jcivelli at chromium.org>'s request for review:
Bug 60637: We should have a Content-Type parser
https://bugs.webkit.org/show_bug.cgi?id=60637

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

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=93151&action=review

I'm going to mark this R+, but other folks should feel free to comment as well.
 Please address the (minor) feedback above before landing.  I presume this is
going to be tested in subsequent MHTML patchs.

> Source/JavaScriptCore/wtf/ASCIICType.h:76
> +    inline bool isASCIIControl(char c) { return (c >= 0 && c < ' ') || c ==
0x7F; }
> +    inline bool isASCIIControl(unsigned short c) { return c < ' ' || c ==
0x7F; }
> +#if !COMPILER(MSVC) || defined(_NATIVE_WCHAR_T_DEFINED)
> +    inline bool isASCIIControl(wchar_t c) { return (c >= 0 && c < ' ') || c
== 0x7F; }
> +#endif
> +    inline bool isASCIIControl(int c) { return (c >= 0 && c < ' ') || c ==
0x7F; }
> +    inline bool isASCIIControl(unsigned c) { return c < ' ' || c == 0x7F; }

This feels like a lot of copy/paste, but I guess that's how this file rolls.


More information about the webkit-reviews mailing list