local folder size limit to extend to 4GB on Mac OS X

RESOLVED FIXED in Thunderbird 3.1b1

Status

defect
--
major
RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: m_kato, Assigned: m_kato)

Tracking

Trunk
Thunderbird 3.1b1
x86
macOS
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird3.1 beta1-fixed)

Details

Attachments

(2 attachments, 2 obsolete attachments)

CString::ToInteger return error in nsParseLocalMessageURI() when msgkey is over PR_INT32_MAX.  So we should use nsCRT::atoll() for nsMsgKey.

Also, atol(over PR_INT32_MAX string) returns 0x7fffffff on Mac OS X.  So we use atoll version for unsigned long (nsMsgKey).
Posted patch patch v1 (obsolete) — Splinter Review
Comment on attachment 420670 [details] [diff] [review]
patch v1

Thx for the patch. Did you want reviews on it?
Attachment #420670 - Flags: review?(bienvenu)
(In reply to comment #2)
> (From update of attachment 420670 [details] [diff] [review])
> Thx for the patch. Did you want reviews on it?

I have waited TryServer build for Linux. It is done, so could you review this?
Comment on attachment 420670 [details] [diff] [review]
patch v1

Thx for the patch. It looks OK. Can you use ParseUint64Str instead of nsCRT::atoll? We're trying to avoid using nsCRT...
(In reply to comment #4)
> (From update of attachment 420670 [details] [diff] [review])
> Thx for the patch. It looks OK. Can you use ParseUint64Str instead of
> nsCRT::atoll? We're trying to avoid using nsCRT...

OK.  Since there is ParseUint64Str() in nsImapUtils.h, should I move it to nsMsgUtils.h.
Posted patch patch v2 (obsolete) — Splinter Review
Attachment #420670 - Attachment is obsolete: true
Attachment #420670 - Flags: review?(bienvenu)
Attachment #421207 - Flags: review?(bienvenu)
thx. We're going to need an xpcshell test for this, I think. I can write it if you aren't able to, but it will take a little time for me to get to it.  The test would be similar to the one I added for the 4GB offline store limit, here - https://bugzilla.mozilla.org/attachment.cgi?id=421461

except that it would be for local mail. I would imagine the test would look something like the following:

test for free disk space > 6GB, just to be safe.
extend the local Inbox to be 3GB using the same approach as I used in the above test (create an output stream, seek to 3GB, and then write some data, and close the file).
Then, set the summary file valid using the local inbox db (you'll need to do this because you've changed the file size, which usually invalidates the db)
Then, append a message to the inbox
Finally, verify that you can stream the message
Attachment #421207 - Attachment is obsolete: true
Attachment #421207 - Flags: review?(bienvenu)
Attachment #421769 - Flags: review?(bienvenu)
Comment on attachment 421769 [details] [diff] [review]
patch v2 with unit test

thx for doing the test. I'd like the test to actually try to stream the message that's past 2GB to make sure that code works - you can use streamMessage with a listener for the data, and just verify that it looks like the original message. I'd also like the test name to not specifically mention the bug #, but be more descriptive - e.g, test_2GBMailboxes.js, something like that. Thx!
Comment on attachment 421769 [details] [diff] [review]
patch v2 with unit test

minusing based on request for a slightly better test.
Attachment #421769 - Flags: review?(bienvenu) → review-
Posted patch patch v3Splinter Review
Attachment #425184 - Flags: review?(bienvenu)
Comment on attachment 425184 [details] [diff] [review]
patch v3

great, thx for the test; I'll try this out as soon as I can (today, I hope).
Comment on attachment 425184 [details] [diff] [review]
patch v3

thx very much, that works. One nit - // get message that msgkey is over 2G.
+
would be better as "get message whose msg key is over 2GB"
Attachment #425184 - Flags: superreview?(bugzilla)
Attachment #425184 - Flags: review?(bienvenu)
Attachment #425184 - Flags: review+
Attachment #425184 - Flags: superreview?(bugzilla) → superreview+
landed
http://hg.mozilla.org/comm-central/rev/dfdc5d1a4278
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
missing target milestone
Target Milestone: --- → Thunderbird 3.0b1
Severity: normal → major
I picked the wrong TM in comment 15.
standard8, pretty sure this landed in 3.1a2/which got renamed to 3.1b1.
bienvenu, what about 3.0.x?  
fixing up flags.
Target Milestone: Thunderbird 3.0b1 → Thunderbird 3.1b1
You need to log in before you can comment on or make changes to this bug.