Bug 60371

Summary: Warning fix on PluginPackage.cpp.
Product: WebKit Reporter: Alexis Menard (darktears) <menard>
Component: Plug-insAssignee: Alexis Menard (darktears) <menard>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ademar, apavlov, commit-queue, darin, eric, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Alexis Menard (darktears)
Reported 2011-05-06 07:15:31 PDT
Warning fix on PluginPackage.cpp.
Attachments
Patch (1.13 KB, patch)
2011-05-06 07:16 PDT, Alexis Menard (darktears) 		no flags
DetailsFormatted DiffDiff
Patch (1.15 KB, patch)
2011-05-06 07:51 PDT, Alexis Menard (darktears) 		no flags
DetailsFormatted DiffDiff
Patch (1.59 KB, patch)
2011-05-10 06:00 PDT, Alexis Menard (darktears) 		no flags
DetailsFormatted DiffDiff
Patch (1.59 KB, patch)
2011-05-10 06:03 PDT, Alexis Menard (darktears) 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Alexis Menard (darktears)
Comment 1 2011-05-06 07:16:41 PDT
Created attachment 92581 [details] Patch
Alexis Menard (darktears)
Comment 2 2011-05-06 07:17:21 PDT
Is there a better way to fix this?
Antonio Gomes
Comment 3 2011-05-06 07:18:14 PDT
Comment on attachment 92581 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=92581&action=review > Source/WebCore/ChangeLog:8 > + Warning fix on conversion from time_t to unsigned. you could say on what platform and compiler it warns :)
Alexis Menard (darktears)
Comment 4 2011-05-06 07:51:49 PDT
Created attachment 92583 [details] Patch
Darin Adler
Comment 5 2011-05-06 09:51:07 PDT
Comment on attachment 92583 [details] Patch This is probably not a good fix. If time_t is not an unsigned then we probably aren’t hashing the whole thing. I think we need a better fix here rather than just quieting the warning.
Alexis Menard (darktears)
Comment 6 2011-05-09 15:51:58 PDT
(In reply to comment #5) > (From update of attachment 92583 [details]) > This is probably not a good fix. If time_t is not an unsigned then we probably aren’t hashing the whole thing. I think we need a better fix here rather than just quieting the warning. Hi Darin, time_t can be a integer (because time() can return -1 when the time can't be read) or a real floating type in some OS. Apparently some embedded systems also have a unsigned 32 bit version of it (so dates prior to 1970 are not represented). unsigned on my 64 bits machine is 4 bytes. The StringHasher::hashMemory<sizeof(hashCodes)>(hashCodes); always takes care of the size of the data going in. So I believe we can just modify hasCodes to store a time_t array or a uint64_t?
Darin Adler
Comment 7 2011-05-09 16:48:34 PDT
(In reply to comment #6) > The StringHasher::hashMemory<sizeof(hashCodes)>(hashCodes); always takes care of the size of the data going in. So I believe we can just modify hasCodes to store a time_t array or a uint64_t? Yes, I think we can change this to be a struct with two members of the appropriate types rather than an array.
Eric Seidel (no email)
Comment 8 2011-05-09 20:40:03 PDT
Comment on attachment 92583 [details] Patch OK.
Eric Seidel (no email)
Comment 9 2011-05-09 20:40:50 PDT
Comment on attachment 92583 [details] Patch Hmm.. Sounds liek Darin would like a differnt fix.
Alexis Menard (darktears)
Comment 10 2011-05-10 06:00:58 PDT
Created attachment 92942 [details] Patch
Alexis Menard (darktears)
Comment 11 2011-05-10 06:03:03 PDT
Created attachment 92943 [details] Patch
WebKit Commit Bot
Comment 12 2011-05-10 08:25:19 PDT
Comment on attachment 92943 [details] Patch Clearing flags on attachment: 92943 Committed r86153: <http://trac.webkit.org/changeset/86153>
WebKit Commit Bot
Comment 13 2011-05-10 08:25:23 PDT
All reviewed patches have been landed. Closing bug.
WebKit Commit Bot
Comment 14 2011-05-10 09:11:18 PDT
The commit-queue encountered the following flaky tests while processing attachment 92943 [details]: http/tests/xmlhttprequest/encode-request-url-2.html bug 51765 (author: ap@webkit.org) The commit-queue is continuing to process your patch.
WebKit Review Bot
Comment 15 2011-05-10 10:20:13 PDT
http://trac.webkit.org/changeset/86153 might have broken Leopard Intel Debug (Tests) The following tests are not passing: fast/lists/003-vertical.html fonts/cursive.html
Alexis Menard (darktears)
Comment 16 2011-05-10 12:13:07 PDT
Anyone with a Leopard Debug build around to help please? I don't have a Mac machine.
Ademar Reis
Comment 17 2011-05-10 13:16:55 PDT
Revision r86153 cherry-picked into qtwebkit-2.2 with commit 2317327 <http://gitorious.org/webkit/qtwebkit/commit/2317327>
Note You need to log in before you can comment on or make changes to this bug.