790043 - nsDeviceStorage compares (& interconverts between) PRTime (signed) and uint64_t (unsigned)

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Daniel Holbert [:dholbert]]

I noticed this build warning:
> dom/devicestorage/nsDeviceStorage.cpp:371:17: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]

The comparison in question there is:
>371     if (msecs < aSince) {
>372       continue;
>373      }
https://mxr.mozilla.org/mozilla-central/source/dom/devicestorage/nsDeviceStorage.cpp#371

where msecs is a PRTime (signed 64-bit) and aSince is uint64_t (unsigned 64-bit)


But if you go up the stack a bit, it looks like aSince originates from a signed PRTime value:
> 1796 nsresult
> 1797 nsDOMDeviceStorage::EnumerateInternal(const JS::Value & aName,
[...]
> 1808   PRTime since = 0;
https://mxr.mozilla.org/mozilla-central/source/dom/devicestorage/nsDeviceStorage.cpp#1797

...and then that's converted to an unsigned uint64_t here, when it's passed to the nsDOMDeviceStorageCursor constructor:

> 1835   nsRefPtr<nsDOMDeviceStorageCursor> cursor = new nsDOMDeviceStorageCursor(win, mPrincipal,
> 1836                                                                            dsf, since);

And then a few levels below that, the unsigned value is compared to a (signed) PRTime value, hence the warning.

I suspect we want to just be using PRTime for all of the "since" / "aSince" / "mSince" variables here.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: fix

Comment 2

[Doug Turner (:dougt)]

Comment on attachment 659848 [details] [diff] [review]
fix

Review of attachment 659848 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm!  thanks

Comment 3

[Daniel Holbert [:dholbert]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fd2e8c30eade

Comment 4

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/fd2e8c30eade