Closed Bug 583618 Opened 11 years ago Closed 10 years ago

Fix PRBool misuses in mailnews

Categories

(MailNews Core :: Backend, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 10.0

People

(Reporter: standard8, Assigned: standard8)

Details

Attachments

(1 file)

As discovered in bug 582523 we are mis-using the PRBool values across the xpconnect boundary.

The case causing test failures is nsMsgHdr::GetIsFlagged, however there's at least one other instance in that file, and I suspect others dotted around.

It would probably also help to do regular static analysis on mailnews, but I'll be filing on a new bug that.
This fixes some of the instances in mailnews/db/msgdb - which should also fix the xpcshell tests.

I've also done a few changes so that where we're returning a PRBool result, the flag checks are a more consistent format.

I also spotted a missing arg check in nsDBFolderInfo so I added that.

I took a look around at other areas of mailnews, but haven't found anything yet. As the majority of the tests pass on trunk, I'm going to assume we're reasonably all right, but getting static analysis done soon will be a priority.
Assignee: nobody → bugzilla
Status: NEW → ASSIGNED
Attachment #462020 - Flags: superreview?(neil)
Attachment #462020 - Flags: review?(neil)
Comment on attachment 462020 [details] [diff] [review]
Fix some nsMsgDatabase errors

>@@ -3999,7 +3999,7 @@ nsresult nsMsgDatabase::GetBooleanProper
> {
>   PRUint32 res;
>   nsresult rv = GetUint32Property(row, propertyName, &res, (PRUint32) defaultValue);
>-  *result = (PRBool) res;
>+  *result = (res != 0);
>   return rv;
> }
Always assuming we called SetBooleanProperty with either PR_TRUE or PR_FALSE, I would have hoped that we would only read back PR_TRUE or PR_FALSE ;-)
Attachment #462020 - Flags: superreview?(neil)
Attachment #462020 - Flags: superreview+
Attachment #462020 - Flags: review?(neil)
Attachment #462020 - Flags: review+
(In reply to comment #2)
> Always assuming we called SetBooleanProperty with either PR_TRUE or PR_FALSE, I
> would have hoped that we would only read back PR_TRUE or PR_FALSE ;-)

I'd hope so too, however, just in case I think its worth changing as well. I have, however, switched it to !!res in that function, as indicated by Mnyromyr on irc.
I checked in http://hg.mozilla.org/comm-central/rev/af27e33d07de as an additional fix with r=Neil over irc. This did the actual xpcshell-test fix.
No longer blocks: fatvals
Shouldn't be necessary to keep this open now that we've switched to bool.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
mark, TM=10?
Target Milestone: --- → Thunderbird 10.0
You need to log in before you can comment on or make changes to this bug.