695235 - XPIDL compiler should enforce size_is parameters being T_U32

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Bobby Holley (:bholley)]

There's a comment in XPConnect saying that the XPIDL compiler enforces this, but pyxpidl doesn't seem to.

The checking we do for this in xpconnect is actually going away in bug 692342, which will land real soon now. So I think it would be very good to get this fixed. Otherwise, we'll have nothing to warn people that they're doing the wrong thing.

Comment 1

[Nathan Froyd [:froydnj]]

Attachment: patch

Check size_is parameter type and also sanity check that the parameter exists.

Surprisingly (?), no problems on Linux/x86-64 with these checks.

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 604525 [details] [diff] [review]
patch

I would prefer that the check went into Param.__init__ to avoid having to loop through all the params.

Comment 3

[Nathan Froyd [:froydnj]]

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #2)
> I would prefer that the check went into Param.__init__ to avoid having to
> loop through all the params.

You can't do that, though, can you?  Because for:

method(... [array, size_is(count)] vector, count...)

When you're __init__'ing count, you don't know that it's a size_is parameter, and when you're __init__'ing vector, you don't have access to the type of count.  So you have to do it in Method...I think.  (Checking for the existence of the size_is parameter requires looping anyway.)

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

mmm, right.  That's what I get for forgetting how this works.

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 604525 [details] [diff] [review]
patch

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

Ok, this is fine.

Comment 6

[Mozilla RelEng Bot]

Autoland Patchset:
	Patches: 604525
	Branch: mozilla-central => try
	Destination: http://hg.mozilla.org/try/pushloghtml?changeset=5f36c384197d
Try run started, revision 5f36c384197d. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=5f36c384197d

Comment 7

[Mozilla RelEng Bot]

Try run for 5f36c384197d is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=5f36c384197d
Results (out of 14 total builds):
    success: 14
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-5f36c384197d

Comment 8

[Dão Gottwald [:dao]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/c2d251ee72d5

Comment 9

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/c2d251ee72d5

Comment 10

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Shouldn't the error message say size_is instead of is_size?

Comment 11

[Nathan Froyd [:froydnj]]

(In reply to Ms2ger from comment #10)
> Shouldn't the error message say size_is instead of is_size?

Doh.  Yes, in both places.