Last Comment Bug 671417 - Incorrect use of PRBool when other types are more appropriate (or vice versa)
: Incorrect use of PRBool when other types are more appropriate (or vice versa)
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Rewriting and Analysis (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla8
Assigned To: Michael Wu [:mwu]
:
Mentors:
: 398581 (view as bug list)
Depends on: 669808
Blocks:
  Show dependency treegraph
 
Reported: 2011-07-13 14:28 PDT by Michael Wu [:mwu]
Modified: 2011-10-16 18:48 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Fix (10.91 KB, patch)
2011-07-13 14:28 PDT, Michael Wu [:mwu]
bzbarsky: review+
ehsan: review+
dwitte: review+
joe: review+
vladimir: review+
luke: review+
mak77: review+
roc: review+
cbiesinger: review+
Details | Diff | Splinter Review

Description Michael Wu [:mwu] 2011-07-13 14:28:58 PDT
Created attachment 545750 [details] [diff] [review]
Fix

Problems discovered by static analysis and patched by hand.

Reviewers this round:

bz:
 content/base/src/nsGenericElement.cpp          |    2 +-
 content/base/src/nsLineBreaker.cpp             |    4 ++--
 docshell/base/nsDocShell.cpp                   |    4 ++--
 layout/xul/base/src/nsListBoxBodyFrame.cpp     |    2 +-
 layout/xul/base/src/nsMenuPopupFrame.h         |    2 +-

ehsan:
 editor/libeditor/html/nsHTMLEditRules.cpp      |    2 +-
 editor/libeditor/html/nsHTMLEditRules.h        |    2 +-
 editor/libeditor/html/nsTableEditor.cpp        |    4 ++--

dwitte:
 extensions/cookie/nsCookiePermission.cpp       |    5 +++--

bholley:
 gfx/thebes/gfxPlatform.cpp                     |    2 +-

vlad:
 intl/uconv/src/nsUTF8ToUnicode.cpp             |    2 +-

luke:
 js/src/jstracer.cpp                            |    2 +-
 js/src/methodjit/StubCalls.cpp                 |    2 +-

biesi:
 netwerk/base/src/nsLoadGroup.cpp               |    4 ++--

mak:
 toolkit/components/places/nsNavHistoryResult.h |    2 +-

roc:
 widget/src/xpwidgets/nsPrintSettingsImpl.cpp   |    2 +-
 widget/src/xpwidgets/nsPrintSettingsImpl.h     |    2 +-
Comment 1 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-07-13 17:41:51 PDT
Comment on attachment 545750 [details] [diff] [review]
Fix

Review of attachment 545750 [details] [diff] [review]:
-----------------------------------------------------------------
Comment 2 Boris Zbarsky [:bz] 2011-07-13 18:20:01 PDT
Comment on attachment 545750 [details] [diff] [review]
Fix

>+++ b/content/base/src/nsLineBreaker.cpp

You need to change nsLineBreaker.h too.

>+++ b/docshell/base/nsDocShell.cpp
>+static PRUint32 gValidateOrigin = 0xffffffff;

Excellent.  We couldn't do this before for some reason....

>+++ b/layout/xul/base/src/nsMenuPopupFrame.h
>+  PRInt8 mConsumeRollupEvent; // Should the rollup event be consumed?

You need to change the constructor so that initialization order matches declaration order.

Also, fix the comment?  Perhaps to say that it's one of nsIPopupBoxObject::ROLLUP_DEFAULT/ROLLUP_CONSUME/ROLLUP_NO_CONSUME ?

r=me on my part with the above nits.
Comment 3 Michael Wu [:mwu] 2011-07-13 18:40:51 PDT
Comment on attachment 545750 [details] [diff] [review]
Fix

Clearing review for the netwerk part because (nssupportsarray)AppendElement doesn't actually return nsresult, as bz points out. Will open another bug to fix that insanity.
Comment 4 Vladimir Vukicevic [:vlad] [:vladv] 2011-07-14 10:41:14 PDT
Comment on attachment 545750 [details] [diff] [review]
Fix

r+ for nsUTF8ToUnicode (can I even r+ that? I know the original/changed code was mine...)
Comment 5 :Ehsan Akhgari 2011-07-14 13:14:02 PDT
Comment on attachment 545750 [details] [diff] [review]
Fix

r=me for the editor parts.
Comment 6 Marco Bonardo [::mak] 2011-07-18 08:25:55 PDT
Comment on attachment 545750 [details] [diff] [review]
Fix

Review of attachment 545750 [details] [diff] [review]:
-----------------------------------------------------------------
Comment 7 Joe Drew (not getting mail) 2011-07-19 08:55:35 PDT
Comment on attachment 545750 [details] [diff] [review]
Fix

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

::: gfx/thebes/gfxPlatform.cpp
@@ +927,4 @@
>  #define INTENT_MIN 0
>  #define INTENT_MAX 3
>  
> +int

This needs to be changed in gfxPlatform.h too.
Comment 8 Michael Wu [:mwu] 2011-07-20 17:20:04 PDT
Comment on attachment 545750 [details] [diff] [review]
Fix

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

::: gfx/thebes/gfxPlatform.cpp
@@ +927,4 @@
>  #define INTENT_MIN 0
>  #define INTENT_MAX 3
>  
> +int

gfxPlatform.h already specifies this as int, AFAICT.
Comment 11 Michael Wu [:mwu] 2011-10-16 18:48:14 PDT
*** Bug 398581 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.