517553 - Remove '=='/'!=' 'PR_FALSE'/'PR_TRUE'

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Jae-Seong Lee-Russo]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20090917 Minefield/3.7a1pre (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20090917 Minefield/3.7a1pre (.NET CLR 3.5.30729)

Enforce Mozilla Coding Style Guide

Do not compare == PR_TRUE or PR_FALSE. Use (x) or (!x) instead. == PR_TRUE, in fact, is *different* from if (x)!

Related to Bug 228448

Reproducible: Didn't try

Comment 1

[Jae-Seong Lee-Russo]

Attachment: Remove '=='/'!=' 'PR_FALSE'/'PR_TRUE' in mozilla-central

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Thanks for the patch. You should find someone who is willed to review your patch.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

As talked on IRC there are tryserver builds available here:
https://build.mozilla.org/tryserver-builds/[e-mail]-1253305907/

Please check if everything works. Afterward you have to ask for review.

Comment 4

[Wan-Teh Chang]

Comment on attachment 401513 [details] [diff] [review]
Remove '=='/'!=' 'PR_FALSE'/'PR_TRUE' in mozilla-central

I will review the NSPR and NSS changes in this patch.

Comment 5

[Jae-Seong Lee-Russo]

Attachment: nspr part

I will break the patch into smaller parts.

Comment 6

[Jae-Seong Lee-Russo]

Attachment: nss part

Comment 7

[Jae-Seong Lee-Russo]

Attachment: widget beos

Comment 8

[Jae-Seong Lee-Russo]

Attachment: widget gtk2

Comment 9

[Jae-Seong Lee-Russo]

Attachment: xpcom

Comment 10

[Jae-Seong Lee-Russo]

Attachment: netwerk

Comment 11

[Jae-Seong Lee-Russo]

Attachment: toolkit

Comment 12

[Jae-Seong Lee-Russo]

Attachment: rdf

Comment 13

[Jae-Seong Lee-Russo]

Attachment: xpconnect

Comment 14

[Jae-Seong Lee-Russo]

Attachment: xptoolkit

Comment 15

[Jae-Seong Lee-Russo]

Attachment: widget windows

Comment 16

[Jae-Seong Lee-Russo]

Attachment: svg

Comment 17

[Jae-Seong Lee-Russo]

Attachment: docshell

Comment 18

[Jae-Seong Lee-Russo]

Attachment: graphics

Comment 19

[Jae-Seong Lee-Russo]

Attachment: style

Comment 20

[Jae-Seong Lee-Russo]

Attachment: intl

Comment 21

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 409856 [details] [diff] [review]
netwerk

+++ b/netwerk/cookie/src/nsCookie.h

I'd prefer you not to change this file... you're storing a PRBool (i.e. a 32-bit integer) in a PRPackedBool (an 8-bit integer). so, if someone passes a value that's not true or false to this function, this risks doing the wrong thing.

Comment 22

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 409853 [details] [diff] [review]
widget beos

+++ b/widget/src/beos/nsDragService.cpp
-                                  aCanDrop == PR_TRUE?"TRUE":"FALSE"));
+                                   aCanDrop?"TRUE":"FALSE"));

while you're here, can you add spaces around ? and : ?

+++ b/widget/src/beos/nsPopupWindow.cpp
+	if (bState && mView && mView->Window() != NULL )

also, can you remove the space before the ) ?

Comment 23

[Christian :Biesinger (don't email me, ping me on IRC)]

out of curiosity, why didn't you use bug 228448 for these patches? (at least the non-toolkit ones)

Comment 24

[Jae-Seong Lee-Russo]

Serge Gautherie suggested to open my own bug blocking bug 228448.

Comment 25

[Jae-Seong Lee-Russo]

Attachment: netwerk, comment 21

Comment 26

[Jae-Seong Lee-Russo]

Attachment: widget beos, comment 22

Comment 27

[Karl Tomlinson (:karlt)]

Comment on attachment 409854 [details] [diff] [review]
widget gtk2

>-  PRBool allocate = (GlobalPrinters::GetInstance()->PrintersAreAllocated() == PR_FALSE);
>+  PRBool allocate = !(GlobalPrinters::GetInstance()->PrintersAreAllocated());

The parentheses are not necessary here.

Comment 28

[Jae-Seong Lee-Russo]

Attachment: widget gtk2, comment 27

Comment 29

[Jae-Seong Lee-Russo]

Attachment: xpfe

Comment 30

[Jae-Seong Lee-Russo]

Attachment: content/smil

Comment 31

[Jae-Seong Lee-Russo]

Attachment: embedding

Comment 32

[Jae-Seong Lee-Russo]

Attachment: dom

Comment 33

[Jae-Seong Lee-Russo]

Attachment: layout

Comment 34

[Jae-Seong Lee-Russo]

Attachment: rdf part 2

Comment 35

[Jae-Seong Lee-Russo]

Attachment: widget

Comment 36

[Jae-Seong Lee-Russo]

Attachment: debugging

Comment 37

[Henri Sivonen (:hsivonen)]

Comment on attachment 410189 [details] [diff] [review]
dom

Looks good to me, but I'm not really a module peer for these files.

Comment 38

[Walter Meinl]

Comment on attachment 409857 [details] [diff] [review]
toolkit

looks good, I don't know anything from BeOS but I've had a look on the BeOS widget patch that got already r+

Comment 39

[Jae-Seong Lee-Russo]

Attachment: check-in needed

I would like check-in this patch before it gets too old.

Comment 40

[Phil Ringnalda (:philor)]

Sadly, the trunk didn't reopen in time, and it's badly bit-rotted.

Comment 41

[Jae-Seong Lee-Russo]

Attachment: check-in please [Checkin: Comment 42]

Comment 42

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

http://hg.mozilla.org/mozilla-central/rev/53308118abed

Comment 43

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Reopening since there are more patches outstanding.

Comment 44

[Jae-Seong Lee-Russo]

Attachment: embedding updated [checkin: Comment 46]

Comment 45

[[not reading bugmail]]

Question: When testing this, did you see any crashes on Windows?  We hit a new crash on last night's nightly with a heavy amount of hits and we're trying to find the regression range.

Comment 46

[Karl Tomlinson (:karlt)]

Comment on attachment 425924 [details] [diff] [review]
embedding updated [checkin: Comment 46]

http://hg.mozilla.org/mozilla-central/rev/e0f83c3ba635

Comment 47

[zmanji]

I am working on this bug. I have created a patch that makes these changes to a single file. If the changes are good, I will continue making them elsewhere.

Comment 48

[zmanji]

Attachment: Fixes in nsprpub/pr/src/md/windows/ntio.c

Comment 49

[Karl Tomlinson (:karlt)]

(In reply to comment #48)
> Fixes in nsprpub/pr/src/md/windows/ntio.c

Many of these changes look similar to changes in attachment 409851 [details] [diff] [review], which hasn't received much attention.
NSS and NSPR are third party libraries from Gecko's perspective.
Patches might receive more attention in an upstream bug report (but they might not).

Comment 50

[Joe Drew (not getting mail)]

Comment on attachment 409868 [details] [diff] [review]
graphics

This patch needs to be updated to latest mozilla-central. Most of these files have been moved, and some no longer exist. However, in general, it looks good, with one caveat that I list below.

>diff --git a/gfx/thebes/public/gfxFont.h b/gfx/thebes/public/gfxFont.h

>         PRUint32 SetCanBreakBefore(PRBool aCanBreakBefore) {
>-            NS_ASSERTION(aCanBreakBefore == PR_FALSE || aCanBreakBefore == PR_TRUE,
>+            NS_ASSERTION(!aCanBreakBefore || aCanBreakBefore,
>                          "Bogus break-before value!");

This is actually checking whether aCanBreakBefore is PR_FALSE or PR_TRUE, so we can't change this assertion.

Comment 51

[Serge Gautherie (:sgautherie)]

(In reply to comment #50)
> This is actually checking whether aCanBreakBefore is PR_FALSE or PR_TRUE, so we
> can't change this assertion.

Yet adding an explicit comment would be an improvement.

Comment 52

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:overholt, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 53

[Andrew McCreight [:mccr8]]

Some patches landed a long time ago in this bug, and PR_TRUE and PR_FALSE are long gone, so let's mark this fixed.