nsBoxObject::SetPropertyAsSupports cannot recognize nsInterfaceHashtable.Init error

RESOLVED FIXED in mozilla13

Status

()

Core
XUL
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: m_kato, Assigned: m_kato)

Tracking

Trunk
mozilla13
Points:
---

Firefox Tracking Flags

(firefox10-)

Details

(Whiteboard: [inbound])

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
Although nsInterfaceHashtable.Init returns bool, nsBoxObject::SetPropertyAsSupports uses NS_FAILED.  NS_FAILED(bool) is always true.

Comment 1

6 years ago
So this used to always test false, and now always tests true?

That's really bad...

Michael, could we write some sort of static analysis to catch other cases like this?
Blocks: 675553
tracking-firefox10: --- → ?
Keywords: regression

Comment 2

6 years ago
(In reply to Boris Zbarsky (:bz) from comment #1)
> So this used to always test false, and now always tests true?
> 
As far as I can tell, this always tested false, and will continue to test false. The underlying implementation (nsTHashtable) returns true for success and false for failure, so the switch to bool cannot change anything as far as I can tell.

> That's really bad...
> 
> Michael, could we write some sort of static analysis to catch other cases
> like this?

MSVC actually tells us if we're trying to test bits in a bool which cannot be anything but a zero.
No longer blocks: 675553

Updated

6 years ago
Keywords: regression

Comment 3

6 years ago
> As far as I can tell, this always tested false, and will continue to test false

Ok, then there's no real issue.  Comment 0 was implying that NS_FAILED(bool) would be true for some reason, so I assumed there was something about the C++ treatment of bool that I was missing....

Comment 4

6 years ago
I did just verify that bool(true & 0x80000000) is false, and likewise for false.  Sorry for the false alarm.  :(

Updated

6 years ago
tracking-firefox10: ? → -
(Assignee)

Comment 5

6 years ago
Created attachment 592595 [details] [diff] [review]
fix
(Assignee)

Updated

6 years ago
Attachment #592595 - Flags: review?(bzbarsky)

Comment 6

6 years ago
Comment on attachment 592595 [details] [diff] [review]
fix

r=me
Attachment #592595 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 7

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/a6361b071a5c
Whiteboard: [inbound]

Comment 8

6 years ago
https://hg.mozilla.org/mozilla-central/rev/a6361b071a5c
Assignee: nobody → m_kato
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in before you can comment on or make changes to this bug.