227135 - TestCallTemplates crashes - erroneous test

Status: VERIFIED FIXED

(Core :: XPCOM, defect)

Description

[Devon Miller]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.5) Gecko/20031007
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.5) Gecko/20031007

This test crashes on a read through an invalid pointer.

The comment here:
http://lxr.mozilla.org/seamonkey/source/xpcom/tests/TestCallTemplates.cpp#26

Indicates the test is not intended to run, only to build.

If that's the way it should work, please add an assert to stop execution
immediately.

I've marked this as critical because a test case should not crash when it is
working correctly!

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Devon Miller]

Attachment: Patch for TestCallTemplates.cpp

This is adds an assertion to TestCallTemplates.cpp to prevent execution of the
test.

Comment 2

[Christian Hamacher]

*** Bug 227133 has been marked as a duplicate of this bug. ***

Comment 3

[Andreas Kunz]

Devon, if you want your patch to be used, you have to request review from one of
the module owners/peers. http://www.mozilla.org/owners.html#XPCOM lists them,
alecf or dougt *might* be responding quickest.

Comment 4

[Devon Miller]

Comment on attachment 136568 [details] [diff] [review]
Patch for TestCallTemplates.cpp

Requesting review by dougt

Comment 5

[Doug Turner (:dougt)]

Comment on attachment 136568 [details] [diff] [review]
Patch for TestCallTemplates.cpp

defer to [e-mail]

Comment 6

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

Comment on attachment 136568 [details] [diff] [review]
Patch for TestCallTemplates.cpp

Use NS_NOTREACHED, and don't use |#ifdef NS_DEBUG|, since the assertion macros
are all no-ops when DEBUG is not defined.  With those changes, r=dbaron.

Comment 7

[Andreas Kunz]

Attachment: updated fix per dbaron

So more like this?

Comment 8

[Andreas Kunz]

Comment on attachment 145511 [details] [diff] [review]
updated fix per dbaron

Requesting r= from dbaron. Review comments addressed, successfully compiled and
tested.

Comment 9

[Andreas Kunz]

Comment on attachment 145511 [details] [diff] [review]
updated fix per dbaron

I think XPCOM is review-only-zone, right?
Not sure if this needs approval, since 1.7 will not use this file. But tests
are built by default and better safe than sorry, so requesting approval1.7.
This is virtually no-risk, just an assertion in a test-file.

Comment 10

[Andreas Kunz]

Comment on attachment 145511 [details] [diff] [review]
updated fix per dbaron

OTOH this can wait until Friday when the 1.7 branch is cut.
Removin approval request.

Comment 11

[Stefan Borggraefe]

Patch checked in.

Comment 12

[Andreas Kunz]

Verifying per lxr, credits for the patch go to Devon Miller.