[Webkit-unassigned] [Bug 15715] Nested XSL stylesheets can produce memory corruption

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=15715


darin at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #22944|review?                     |review-
               Flag|                            |




------- Comment #24 from darin at apple.com  2008-08-24 15:14 PDT -------
(From update of attachment 22944)
Looks OK. I have a few thoughts and questions.

Mark Rowe requested a regression test; I'd like to see the patch include one.

Since parentStyleSheet() is never used outside the XSLStyleSheet class, I don't
think we need to define a function. If later we decide it's needed, that's OK,
but it's slightly nicer to have tighter encapsulation and know no one outside
the class looks at m_parentStyleSheet.

What guarantees that m_parentStyleSheet is not left pointing to a deleted
object?

+    void setParentStyleSheet(XSLStyleSheet* parent) {
+       m_parentStyleSheet = parent; 
+       if (parent)
+          setOwnerDocument(parent->ownerDocument());
+    }

The opening brace is supposed to go on a separate line. This function is also
getting long enough that it might be better to not put it inline. Our normal
approach is to only use inlines for completely trivial functions unless there's
a specific performance motivation for doing so.

Are there any callers left for XSLStyleSheet::setOwnerDocument()? If not,
perhaps we should remove it.

I'm going to say review- since I'm hoping for a patch with a regression test.
In a pinch I suppose we can land this without one. Let me know if you'd like to
do that -- you should even feel re-mark this patch for review if you think this
should go in as-is even considering my comments.


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.