Last Comment Bug 782608 - Use NS_FAILED instead of boolean test (image/)
: Use NS_FAILED instead of boolean test (image/)
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: ImageLib (show other bugs)
: Trunk
: All All
: -- minor (vote)
: mozilla17
Assigned To: Aryeh Gregor (:ayg) (next working March 28-April 26)
:
: Milan Sreckovic [:milan]
Mentors:
Depends on:
Blocks: nsresult-enum-class
  Show dependency treegraph
 
Reported: 2012-08-14 05:18 PDT by Aryeh Gregor (:ayg) (next working March 28-April 26)
Modified: 2012-08-16 17:58 PDT (History)
1 user (show)
ayg: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (1.11 KB, patch)
2012-08-14 05:22 PDT, Aryeh Gregor (:ayg) (next working March 28-April 26)
justin.lebar+bug: review+
Details | Diff | Splinter Review

Description Aryeh Gregor (:ayg) (next working March 28-April 26) 2012-08-14 05:18:16 PDT

    
Comment 1 Aryeh Gregor (:ayg) (next working March 28-April 26) 2012-08-14 05:22:13 PDT
Created attachment 651713 [details] [diff] [review]
Patch

Treating an nsresult as a bool is incorrect, because there are success codes other than NS_OK (although they're rarely used!).  Without this patch, the file will fail to compile when nsresult becomes an enum class, because enum classes don't implicitly convert to other types.

This patch will only change behavior if something was using some value in the range 0 < x < 0x80000000 as an nsresult, and expecting it to be treated as an error.  This is probably not the case here, but I didn't check every usage of this macro in the file -- there are a lot.  Generally this only happens with things that grossly abuse nsresult, like some nsISupportsArray methods that return a boolean when they're declared to return an nsresult.
Comment 2 Justin Lebar (not reading bugmail) 2012-08-14 08:54:42 PDT
> but I didn't check every usage of this macro in the file -- there are a lot.

All the uses look fine to me, although I didn't go and check that the functions involved return what they seem to return.

This will likely burn a test if someone does something wrong, so I'm OK with it.
Comment 3 Aryeh Gregor (:ayg) (next working March 28-April 26) 2012-08-16 05:59:36 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/e0f7d474fb52

Green try: https://tbpl.mozilla.org/?tree=Try&rev=da4047717ef7
Comment 4 Ryan VanderMeulen [:RyanVM] 2012-08-16 17:58:09 PDT
https://hg.mozilla.org/mozilla-central/rev/e0f7d474fb52

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