Closed
Bug 986976
Opened 11 years ago
Closed 11 years ago
do_QueryInterface abuse in various .mm files
Categories
(Core :: Widget: Cocoa, defect)
Core
Widget: Cocoa
Tracking
()
RESOLVED
FIXED
mozilla31
People
(Reporter: neil, Assigned: smichaud)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
smichaud
:
review+
|
Details | Diff | Splinter Review |
The code
nsCOMPtr<nsPrintSettingsX> printSettingsX(do_QueryInterface(aPS));
which appears several times in various .mm files is an abuse of do_QueryInterface, since nsPrintSettingsX is not an interface, so what happens is that aPS gets reinterpreted as an nsPrintSettingsX*.
You may want to add an IID to nsPrintSettingsX which would allow you to write:
nsRefPtr<nsPrintSettingsX> printSettingsX(do_QueryObject(aPS));
Assignee | ||
Comment 1•11 years ago
|
||
How embarrassing :-(
> nsCOMPtr<nsPrintSettingsX> printSettingsX(do_QueryInterface(aPS));
These were originally as follows, until bug 520494 eliminated the nsIPrintSettingsX interface:
nsCOMPtr<nsIPrintSettingsX> printSettingsX(do_QueryInterface(aPS));
I'll come up with a patch.
Assignee | ||
Comment 2•11 years ago
|
||
> what happens is that aPS gets reinterpreted as an nsPrintSettingsX*
I'm surprised there isn't a compiler error, or at least a warning. Their absence is probably itself a bug.
Reporter | ||
Comment 3•11 years ago
|
||
Indeed, I discovered the problem trying to make it a compiler error (see bug 514280). I don't know if it's possible to make it a compiler warning though.
Assignee | ||
Comment 4•11 years ago
|
||
Were you looking for something like this?
Reporter | ||
Comment 5•11 years ago
|
||
Comment on attachment 8396049 [details] [diff] [review]
Possible fix
This seems fine although I don't think you needed to expand the NS_IMPL_ISUPPORTS_INHERITED1 macro (a peruse through the tree shows it expanded in the case of ambiguous nsISupports inheritance or having to support class info).
I checked that compiles in conjunction with the rest of my changes from bug 514280 (my try push didn't run any tests though because it includes some workarounds for the related dependent bugs just to get the code to compile.)
Attachment #8396049 -
Flags: review?(neil) → review+
Assignee | ||
Comment 6•11 years ago
|
||
> I don't think you needed to expand the NS_IMPL_ISUPPORTS_INHERITED1
I came to the same conclusion overnight. I'll land the patch without the expansion.
Thanks.
Assignee | ||
Comment 7•11 years ago
|
||
Don't expand the NS_IMPL_ISUPPORTS_INHERITED1 macro. Carrying over Neil's r+.
Attachment #8396049 -
Attachment is obsolete: true
Attachment #8396474 -
Flags: review+
Assignee | ||
Comment 8•11 years ago
|
||
Comment on attachment 8396474 [details] [diff] [review]
Fix rev1 (don't expand NS_IMPL_ISUPPORTS_INHERITED1)
Landed on mozilla-inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1f924b14ed31
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in
before you can comment on or make changes to this bug.
Description
•