583618 - Fix PRBool misuses in mailnews

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Mark Banner (:standard8)]

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.

Comment 1

[Mark Banner (:standard8)]

Attachment: Fix some nsMsgDatabase errors

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.

Comment 2

[[e-mail]]

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 ;-)

Comment 3

[Mark Banner (:standard8)]

(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.

Comment 4

[Mark Banner (:standard8)]

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.

Comment 5

[Mark Banner (:standard8)]

Shouldn't be necessary to keep this open now that we've switched to bool.

Comment 6

[Wayne Mery (:wsmwk)]

mark, TM=10?