253941 - It's possible to assign into a nsCOMPtr from an incompatible type

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

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

nsCOMPtr<imgIContainer> mContainer;

  nsCOMArray<imgIRequest> mCursorArray; // [inherited] The specified URL values.
Takes precedence over mCursor.

Now doing mContainer = mCursorArray[i]; works. mContainer is null afterwards.

it is rather unexpected that this compiles despite the different types. it seems
to do a QI.

Is this desired behaviour?

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Is this specific to nsCOMArray?

Comment 2

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

(this was with gcc version 3.3.3 (SuSE Linux))

This following code also compiles:
      nsCOMPtr<imgIContainer> c;
      imgIRequest* r = ui->mCursorArray[i];
      c = r;

So, doesn't look specific to nsCOMArray.

Comment 3

[:deanis74]

From bug 264987, this code also compiled:

  nsIDocument* document = content->GetOwnerDoc();
  nsCOMPtr<nsIDOMHTMLDocument> htmlDoc(document);

Comment 4

[[e-mail]]

Using gcc 2.9.6 the following code does not compile:
nsISupportsPrimitive p;
nsCOMPtr<nsISupportsString> s = p;

Comment 5

[[e-mail]]

Or indeed even with p correctly declared as a pointer.
However it will compile with the assignment changed to a constructor.

Comment 6

[[e-mail]]

At least, I have scrollback claiming it compiled...

Comment 7

[[e-mail]]

Ok, so having saved my test file correctly the problem is the implicit
nsQueryInterface(nsISupports*) constructor. Making it explicit fixes it.
I didn't look for other constructors that needed fixing.

Comment 8

[[e-mail]]

Attachment: Proposed patch

I couldn't find any other implicit conversions that could get abused.

Comment 9

[Scott Collins]

Comment on attachment 163005 [details] [diff] [review]
Proposed patch

r=scc

make sure you build a whole tree with this before you check it in, in case
there has been abuse

Comment 10

[Justin Wood (:Callek)]

I personally would have opted for a configure rule to/in nspr for a PR_EXPLICIT
or some such, so that compilers which do not recognize explicit when built with
Mozilla will still treat this correctly, but otherwise good work.

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

We use |explicit| all over the place, and nscore.h already handles compilers
that don't support it by defining it.

Comment 12

[Justin Wood (:Callek)]

Thanks for the clarification, I must have just missed those explicit cases...

Comment 13

[[e-mail]]

Fix checked in (again... I checked it in then backed it out when I realized I
hadn't built accessibility, but then bz fixed my regressions anyway, thanks!)