334007 - Schema Validation uses getter_AddRefs wrong, can cause crash

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[Doron Rosenberg (IBM)]

this fixes the crash in the first testcase in bug 333113

Comment 1

[Doron Rosenberg (IBM)]

Attachment: patch

Stupid me, was doing foo->bar(getter_AddRefs(foo)) in two places inside the same method.

Also fix 2 warnings.

Comment 2

[aaronr]

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

>? extensions/schema-validation/src/temp
>Index: extensions/schema-validation/src/nsSchemaValidator.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/schema-validation/src/nsSchemaValidator.cpp,v
>retrieving revision 1.24
>diff -u -r1.24 nsSchemaValidator.cpp
>--- extensions/schema-validation/src/nsSchemaValidator.cpp	10 Apr 2006 16:51:32 -0000	1.24
>+++ extensions/schema-validation/src/nsSchemaValidator.cpp	14 Apr 2006 16:31:19 -0000
>@@ -3828,7 +3828,7 @@
>   rv = aSchemaParticle->GetMaxOccurs(&maxOccurs);
>   NS_ENSURE_SUCCESS(rv, rv);
> 
>-  nsCOMPtr<nsIDOMNode>leftOvers;
>+  nsCOMPtr<nsIDOMNode> leftOvers, tmpNode;
> 
>   switch(particleType) {
>     case nsISchemaParticle::PARTICLE_TYPE_ELEMENT: {
>@@ -3845,7 +3845,8 @@
> 
>         // if not an element node, skip
>         if (nodeType != nsIDOMNode::ELEMENT_NODE) {
>-          leftOvers->GetNextSibling(getter_AddRefs(leftOvers));
>+          leftOvers->GetNextSibling(getter_AddRefs(tmpNode));
>+          leftOvers = tmpNode;
>           continue;
>         }
> 

nit: I'd suggest nulling out tmpNode after you assign it into leftOvers since you never check return codes.  That way a junk tmpNode can't make it any farther.  But not that big of a deal.

>@@ -3860,8 +3861,10 @@
>           NS_ENSURE_SUCCESS(rv, rv);
> 
>           // set rest to the next element if node is valid
>-          if (isValid)
>-            leftOvers->GetNextSibling(getter_AddRefs(leftOvers));
>+          if (isValid) {
>+            leftOvers->GetNextSibling(getter_AddRefs(tmpNode));
>+            leftOvers = tmpNode;
>+          }
> 

same here.

r=me

Comment 3

[Doron Rosenberg (IBM)]

allan, if you r+, could you check this into trunk/branches since you are first up? thanks!

Comment 4

[Allan Beaufour]

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

with Aaron's comments fixed, r=me too

Comment 5

[Allan Beaufour]

Fixed on trunk.

Comment 6

[Allan Beaufour]

Attachment: Checked in version

Comment 7

[Allan Beaufour]

Checked in on 1_8_0 too. So hopefully, this will fix the crash in the release XPI?

Comment 8

[Doron Rosenberg (IBM)]

(In reply to comment #7)
> Checked in on 1_8_0 too. So hopefully, this will fix the crash in the release
> XPI?
> 

Hopefully it will be included in today's nightly xpis