[Webkit-unassigned] [Bug 41348] Remove global variables from XSLTProcessorLibxslt.cpp

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jul 2 10:17:41 PDT 2010


Alexey Proskuryakov <ap at webkit.org> changed:

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

--- Comment #8 from Alexey Proskuryakov <ap at webkit.org>  2010-07-02 10:17:41 PST ---
(From update of attachment 60362)
+static HashSet<XSLTProcessor*> globalXSLTProcessors;
+static HashSet<XSLStyleSheet*> globalXSLStyleSheets;

I don't think this will even build on Mac - there is a step in build script that verifies that there are no global destructors. Our usual pattern is to have a static function with a DEFINE_STATIC_GLOBAL macro inside.

+        xsltTransformContextPtr context = reinterpret_cast<xsltTransformContextPtr>(ctxt);
+        XSLTProcessor* processor = reinterpret_cast<XSLTProcessor*>(context->_private);

Didn't static_cast work?

+                fprintf(stderr, "Expected a registred XSLTProcessor pointer in the libxslt "
+                        "_private field but found something else: %p\n", context->_private);

This is not a message that can help the author of an application using WebKit much. We need to give them pointers they can use to work around the problem.

Something like "WebKit XSLT document loader was called with unknown transformation context." might be more helpful.

Looking at just this code, it seems that it can be simplified a little by storing transformation contexts instead of processors/stylesheets. But it's difficult to get to the context from where _private is set. Maybe it can be structured differently, although I don't have a concrete solution.

+    Node* m_sourceNode;

Ideally, this would have a comment explaining why this can't become a dangling pointer.

r- because of HashSet destructor issue.

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

More information about the webkit-unassigned mailing list