Bug 60371 – Warning fix on PluginPackage.cpp.

• Status: RESOLVED
• Resolution: FIXED
• Component: Plug-ins

[Alexis Menard (darktears)]

Warning fix on PluginPackage.cpp.

[Alexis Menard (darktears)]

Created attachment 92581 [details]
Patch

[Alexis Menard (darktears)]

Is there a better way to fix this?

[Antonio Gomes]

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)]

Created attachment 92583 [details]
Patch

[Darin Adler]

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)]

(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]

(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 on attachment 92583 [details]
Patch

OK.

[Eric Seidel (no email)]

Comment on attachment 92583 [details]
Patch

Hmm.. Sounds liek Darin would like a differnt fix.

[Alexis Menard (darktears)]

Created attachment 92942 [details]
Patch

[Alexis Menard (darktears)]

Created attachment 92943 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 92943 [details]
Patch

Clearing flags on attachment: 92943

Committed r86153: <http://trac.webkit.org/changeset/86153>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 92943 [details]:

http/tests/xmlhttprequest/encode-request-url-2.html bug 51765 (author: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Review Bot]

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)]

Anyone with a Leopard Debug build around to help please? I don't have a Mac machine.

[Ademar Reis]

Revision r86153 cherry-picked into qtwebkit-2.2 with commit 2317327 <http://gitorious.org/webkit/qtwebkit/commit/2317327>