Closed
Bug 123402
Opened 23 years ago
Closed 22 years ago
PRTime abuse
Categories
(MailNews Core :: Backend, defect)
MailNews Core
Backend
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: timeless, Assigned: naving)
Details
(Keywords: 64bit, Whiteboard: [verifiedish1])
Attachments
(1 file, 2 obsolete files)
2.08 KB,
patch
|
timeless
:
review+
Bienvenu
:
superreview+
jesup
:
approval+
|
Details | Diff | Splinter Review |
/home/timeless/mozilla/mailnews/base/util/nsMsgDBFolder.cpp:1398: warning: decimal integer constant is so large that it is unsigned http://lxr.mozilla.org/mozilla/source/mailnews/base/util/nsMsgDBFolder.cpp 65 #define oneHour 3600000000 1394 PRTime timeOfLastPurgeCheck = gtimeOfLastPurgeCheck; 1398 timeOfLastPurgeCheck += oneHour; http://lxr.mozilla.org/nspr/source/nsprpub/pr/include/prtime.h#77 77 typedef PRInt64 PRTime; http://lxr.mozilla.org/nspr/ident?i=PRInt64 Defined as a struct type in: * nsprpub/pr/include/prtypes.h, line 340 [this only happens when !HAVE_LONG_LONG, but this is possible] $ cat /tmp/abcd.cpp struct a{short b; short c;}; the current code as seen by a int main(){ a d; d=201; return 0; $ g++ /tmp/abcd.cpp /tmp/abcd.cpp: In function `int main()': /tmp/abcd.cpp:5: no match for `a & = int' /tmp/abcd.cpp:1: candidates are: struct a & a::operator =(const a &) What should the code do? use LL_ADD and LL_INIT, write oneHour as 0xD693A400
Comment 1•23 years ago
|
||
correcting component - I don't think writing the constant in equivalent hex is going to make any difference, cc'ing Navin, who I think wrote this code. There's a way to indicate that you want a 64 bit constant long, but I don't remember off the top of my head what it is.
Status: UNCONFIRMED → NEW
Component: Mail Database → Mail Back End
Ever confirmed: true
Assignee | ||
Comment 2•23 years ago
|
||
taking, my code, i will have to find out more..
Assignee: bienvenu → naving
indeed switching to hex isn't as important as using the LL_ macros. The point of this bug is that you aren't guaranteed that PRTime is a 64bit number. I think you can use PR_UINT32(36...) if you prefer that over hex.
Keywords: 64bit
Assignee | ||
Comment 4•22 years ago
|
||
Sean was here this morning and he was prompted to auto-compact twice in a row within seconds interval. That should not have happened. He lost two folders!! Basically nsMsgDBFolder::AutoCompact() throws that prompt. looks like gTimeOfLastPurgeCheck may not have been set correctly. So I changed it to set gTimeOfLastPurgeCheck much earlier and use those macros in case there could have been an overflow. I am still looking into nsMsgFolderCompactor code.
Assignee | ||
Comment 5•22 years ago
|
||
cavin, david. Can I get reviews ? thx
Comment on attachment 94417 [details] [diff] [review] proposed fix >- PRTime timeNow = PR_Now(); //time in microsec this was correct unless you're intentionally truncating >+ PRTime timeNow; >+ LL_I2L(timeNow,PR_Now()); //time in microseconds this is wrong unless you're trying to do something strange http://lxr.mozilla.org/nspr/source/nsprpub/pr/include/prtime.h#177 177 NSPR_API(PRTime) 178 #endif 179 PR_Now(void); http://lxr.mozilla.org/nspr/source/nsprpub/pr/include/prlong.h#166 166 ** LL_I2L Convert signed to 64 bit
Attachment #94417 -
Flags: needs-work+
Comment 7•22 years ago
|
||
I believe timeless is correct.
Assignee | ||
Comment 8•22 years ago
|
||
casting PRInt64 to PRInt64 is harmless as far as I can see (though it may not be needed)
Assignee | ||
Comment 9•22 years ago
|
||
ok made that change.
Attachment #94417 -
Attachment is obsolete: true
Reporter | ||
Comment 10•22 years ago
|
||
Comment on attachment 94423 [details] [diff] [review] patch >+ PRTime timeNow = PR_Now(); //time in microseconds >+ PRTime timeAfterOneHourOfLastPurgeCheck; ... >+ if (timeAfterOneHourOfLastPurgeCheck < timeNow && prompt) i suck please use LL_CMP http://lxr.mozilla.org/nspr/ident?i=LL_CMP so if (LL_CMP(timeAfterOneHourOfLastPurgeCheck, <, timeNow) && prompt) I believe that's everything. I'm very sorry that I missed this.
Attachment #94423 -
Flags: review+
Attachment #94423 -
Flags: needs-work+
Assignee | ||
Comment 11•22 years ago
|
||
I changed it to LL_CMP in my tree.
Assignee | ||
Comment 12•22 years ago
|
||
Attachment #94423 -
Attachment is obsolete: true
Comment 13•22 years ago
|
||
Comment on attachment 94518 [details] [diff] [review] patch with LL_CMP sr=bienvenu
Attachment #94518 -
Flags: superreview+
Attachment #94518 -
Flags: review+
Assignee | ||
Comment 14•22 years ago
|
||
fixed
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
QA Contact: esther → stephend
Verified FIXED using my local source tree.
Status: RESOLVED → VERIFIED
Updated•22 years ago
|
Comment 16•22 years ago
|
||
Comment on attachment 94518 [details] [diff] [review] patch with LL_CMP a=rjesup@wgate.com for 1.0 branch. Please change mozilla1.0.2+ to fixed1.0.2 when checked in
Attachment #94518 -
Flags: approval+
Updated•22 years ago
|
Keywords: mozilla1.0.2+
Verified on the branch: http://lxr.mozilla.org/mozilla1.0/source/mailnews/base/util/nsMsgDBFolder.cpp
Keywords: fixed1.0.2 → verified1.0.2
Updated•22 years ago
|
Whiteboard: [ish1+]
Assignee | ||
Updated•22 years ago
|
Whiteboard: [ish1+] → [fixedish1]
Assignee | ||
Updated•22 years ago
|
Whiteboard: [fixedish1] → [fixedish1][ish1+]
Whiteboard: [fixedish1][ish1+] → [fixedish1][verifiedish1+]
Whiteboard: [fixedish1][verifiedish1+] → [verifiedish1]
Updated•20 years ago
|
Product: MailNews → Core
Updated•16 years ago
|
Product: Core → MailNews Core
You need to log in
before you can comment on or make changes to this bug.
Description
•