If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Replace mdb_err by nsresult

RESOLVED FIXED in Thunderbird 30.0

Status

MailNews Core
Database
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: standard8, Assigned: Cykesiopka)

Tracking

Trunk
Thunderbird 30.0

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

105.56 KB, patch
standard8
: review+
Details | Diff | Splinter Review
(Reporter)

Description

5 years ago
mdb_err is typedefed to nsresult, we should just drop the mdb_err and avoid confusion.

I suspect this can largely be scripted.
(Assignee)

Comment 1

4 years ago
Created attachment 8355838 [details] [diff] [review]
bug801553_v1.patch
Attachment #8355838 - Flags: review?(mbanner)
(Reporter)

Comment 2

4 years ago
Comment on attachment 8355838 [details] [diff] [review]
bug801553_v1.patch

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

Thanks for the patch. Looks good, r=Standard8. There's one minor change, but I'll do that for you on landing as I've got it locally already.

::: db/mork/src/morkZone.cpp
@@ +499,5 @@
>      outErr = ev->AsErr();
>    }
>    else
> +    // XXX 1 is not a valid nsresult
> +    outErr = static_cast<nsresult>(1);

I've just had a look through and the majority of callers don't actually use the return result. Additionally orkinHeap.cpp already returns morkEnv_kOutOfMemoryError so I think it is safe to return that here.
Attachment #8355838 - Flags: review?(mbanner) → review+
(Reporter)

Comment 3

4 years ago
Comment on attachment 8355838 [details] [diff] [review]
bug801553_v1.patch

Ah sorry about this, I just realized I didn't actually have the patch applied when I compiled this. This broke the build, an there's errors about unknown type 'mdb_err' in

mailnews/addrbook/src/nsAddrDatabase.cpp

I suspect there will be some in mailnews/base as well.
Attachment #8355838 - Flags: review+ → review-

Updated

4 years ago
Assignee: nobody → cykesiopka.bmo
(Assignee)

Comment 4

4 years ago
(In reply to Mark Banner (:standard8) (away until 3 Mar) from comment #3)
> Comment on attachment 8355838 [details] [diff] [review]
> bug801553_v1.patch
> 
> Ah sorry about this, I just realized I didn't actually have the patch
> applied when I compiled this. This broke the build, an there's errors about
> unknown type 'mdb_err' in
> 
> mailnews/addrbook/src/nsAddrDatabase.cpp
> 
> I suspect there will be some in mailnews/base as well.

Well, that's embarrassing...
Not sure how I managed to miss them.
(Assignee)

Comment 5

4 years ago
Created attachment 8381944 [details] [diff] [review]
bug801553_v2.patch

This should hopefully be all of them this time.

Local clobber builds of Thunderbird and Seamonkey with this patch applied work for me at least.
Attachment #8355838 - Attachment is obsolete: true
Attachment #8381944 - Flags: review?(standard8)
(Reporter)

Comment 6

4 years ago
Comment on attachment 8381944 [details] [diff] [review]
bug801553_v2.patch

Looks great. Thanks.
Attachment #8381944 - Flags: review?(standard8) → review+
(Assignee)

Comment 7

4 years ago
Thanks for the review.
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/614dbfb6656f
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 30.0
You need to log in before you can comment on or make changes to this bug.