Bug 47248 - WTF class CString confuses unsigned and size_t
Summary: WTF class CString confuses unsigned and size_t
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Chris Evans
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-10-05 22:44 PDT by Chris Evans
Modified: 2010-10-06 23:57 PDT (History)
3 users (show)

See Also:


Attachments
Patch (3.25 KB, patch)
2010-10-05 22:50 PDT, Chris Evans
no flags Details | Formatted Diff | Diff
Patch (3.28 KB, patch)
2010-10-05 23:20 PDT, Chris Evans
cevans: review-
Details | Formatted Diff | Diff
Patch for EWS try bot (3.84 KB, patch)
2010-10-06 15:35 PDT, Chris Evans
no flags Details | Formatted Diff | Diff
Patch (3.85 KB, patch)
2010-10-06 23:05 PDT, Chris Evans
no flags Details | Formatted Diff | Diff
Patch (3.85 KB, patch)
2010-10-06 23:06 PDT, Chris Evans
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Evans 2010-10-05 22:44:29 PDT
unsigned != size_t on a 64-bit build.
Theoretically, a CString can have e.g. a 5GB string buffer on a 64-bit platform. There are various errors that would happen in such a case, such as "unsigned CString::length()" truncating the underlying size.

Patch forthcoming.
Comment 1 Chris Evans 2010-10-05 22:50:42 PDT
Created attachment 69889 [details]
Patch
Comment 2 David Levin 2010-10-05 23:04:29 PDT
Comment on attachment 69889 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=69889&action=review

> JavaScriptCore/wtf/MD5.cpp:79
> +    ASSERT_WITH_MESSAGE(actual == expected, "input:%s[%zu] actual:%s expected:%s", input.data(), input.length(), actual.data(), expected.data());

Does Visual Studio 2005 support this format?
Comment 3 David Levin 2010-10-05 23:17:01 PDT
Adding myself to cc list.
Comment 4 Chris Evans 2010-10-05 23:19:01 PDT
Good point. I did grep for '%zu' and got hits in dom/Node.cpp, but it seems like they are behind some heavy-duty define.

The safest in-use paradigm seems to be usage of %lu plus a cast to unsigned long (e.g. DRT; WebSocketChannel.cpp; IconDatabase.cpp).
Comment 5 Eric Seidel (no email) 2010-10-05 23:20:03 PDT
Attachment 69889 [details] did not build on mac:
Build output: http://queues.webkit.org/results/4182094
Comment 6 Chris Evans 2010-10-05 23:20:50 PDT
Created attachment 69893 [details]
Patch
Comment 7 David Levin 2010-10-05 23:27:49 PDT
Comment on attachment 69893 [details]
Patch

r+ if you fix the export issue for OSX and Windows (-- Sometime people submit the change with the OSX symbol and then get the symbol for windows for the build error on the buildbots).
Comment 8 Chris Evans 2010-10-05 23:34:39 PDT
Thanks -- I'll learn about the magic exports on Mac and Windows tomorrow :)
Comment 9 Chris Evans 2010-10-06 15:35:19 PDT
Created attachment 70000 [details]
Patch for EWS try bot
Comment 10 Chris Evans 2010-10-06 15:42:26 PDT
Comment on attachment 70000 [details]
Patch for EWS try bot

Hmm - using r? to try and spur the bots into action
Comment 11 Chris Evans 2010-10-06 23:05:07 PDT
Created attachment 70038 [details]
Patch
Comment 12 Chris Evans 2010-10-06 23:06:41 PDT
Created attachment 70039 [details]
Patch
Comment 13 WebKit Commit Bot 2010-10-06 23:57:16 PDT
Comment on attachment 70039 [details]
Patch

Clearing flags on attachment: 70039

Committed r69277: <http://trac.webkit.org/changeset/69277>
Comment 14 WebKit Commit Bot 2010-10-06 23:57:22 PDT
All reviewed patches have been landed.  Closing bug.