70855 - Add NSPR logging to moz module, do something for standalone

Status: VERIFIED FIXED

(Core :: XSLT, defect, P5)

Description

[Axel Hecht]

I started adding simple XPCOM logging, so we can find out which stylesheets
are processed, and which XPath expressions are used.
I would like to extend this to the XPath lexer and parser.
The token list for the lexer, and some scetch of the generated objects for the
parser.

Use the stuff by setting NSPR_LOG_MODULES to "XSLT:5;XPATH:5" to get all 
current logging. I suggest XPATH_PARSER and XPATH_LEXER for the other logs.

I added source/base/TxLog.h, which is used to define the macros for the 
standalone, and currently does nothing. It'd cause to much flood, IMHO.
But it's there, if someone wants to use it.

Code and patch coming in a minute.

Axel

Comment 1

[Axel Hecht]

Attachment: header for standalone

Comment 2

[Axel Hecht]

Attachment: adding logging to XSLTProcessor::TransformDocument and ProcessorState::getExpr

Comment 3

[Axel Hecht]

XPCOM logging is dying. Switch over to NSPR.

So, there are a few issues:

First, what to do for standalone.
There are 4 different levels of logging in NSPR, should we try to mimic that?
Runtime with environment var? or compile time with define?
Output should simply go to stderr, no direct support for files, IMHO. We still
have redirects.

When should we get the PRLogModuleInfo structs? For each logging, or should we
cache that in globals? Module might need refcounting again.

I want to put all the switching stuff to TxLog.h, so we just have to include 
that, and use the macros defined there for what we need.

Getting a bit of life here, hopefully

Axel

Comment 4

[Axel Hecht]

Attachment: nspr logging, first attempt

This is a whacky attempt to get logging again.
I added logging for both module and standalone, starting with the two spots
that I used for getting a grip on the docbook foo.
I just printf without levels for standalone, which is a start. I don't really
want to bother with runtime levels and environment variables. At least not
in this phase.
I'm not sure if we should typedef our own symbols for the levels, I really
don't
dig the names of the nspr ones.

Comment 5

[Axel Hecht]

Attachment: updated to the current tree

peterv was sobbing. So I make him suffer. harhar.
logging works for template matching and call-template, so you get the basic
flow of the transform

Comment 6

[Axel Hecht]

Attachment: builds and runs

sorry, I didn't bother building it.
I had to remove aContext, 'cause that died in the meantime and adjust the
TX_LOG_CREATE position.

Comment 7

[Peter Van der Beken [:peterv]]

Comment on attachment 79793 [details] [diff] [review]
builds and runs

r=peterv.

Comment 8

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Comment on attachment 79793 [details] [diff] [review]
builds and runs

>Index: build/XSLTProcessorModule.cpp
>===================================================================
>Index: source/base/TxLog.h
>===================================================================
>Index: source/xslt/ProcessorState.cpp
>===================================================================
>+    if (matchTemplate) {
>+        PR_LOG(txLog::xslt, PR_LOG_DEBUG,
>+               ("MatchTemplate, Pattern %s, Mode %s, Stylesheet %s, " \
>+                "Node %s\n",
>+                ((Element*)matchTemplate)->getAttribute("match").toCharArray(),
>+                aMode.toCharArray(),
>+                matchTemplate->getBaseURI().toCharArray(),
>+                aNode->getNodeName().toCharArray()));
>+    }
>+    else {
>+        PR_LOG(txLog::xslt, PR_LOG_DEBUG,
>+               ("No match, Node %s, Mode %s\n", 
>+                aNode->getNodeName().toCharArray(),
>+                aMode.toCharArray()));
>+    }

You might want to put that whole block inside #ifdef PR_LOGGING, that
way you would save one compare in release code. See also below.

>Index: source/xslt/XSLTProcessor.cpp
>===================================================================
>+                        PR_LOG(txLog::xslt, PR_LOG_DEBUG,
>+                               ("CallTemplate, Name %s, Stylesheet %s\n",
>+                                templateName.toCharArray(),
>+                                xslTemplate->getBaseURI().toCharArray()));

Both here and above you call toCharArray(), which allocates memory, yet you
don't free it. Please fix that, by using something like:

#ifdef PR_LOGGING
  {
    // get all toCharArray()'s into temp variables
    // do logging
    // free temp variables
  }
#endif // PR_LOGGING

With the memory leaks fixed sr=heikki

Comment 9

[Axel Hecht]

Comment on attachment 79793 [details] [diff] [review]
builds and runs

patch checked in.
There's no need to get this on the branch, but I'll leave the bug open so that
we can add more logging information.

stylesheet import, include come into my mind, apply-imports. Others?

Comment 10

[Axel Hecht]

Attachment: do logging for stylesheet compilation, start and end of main,import,include

Comment 11

[Axel Hecht]

attachement 119854 is in. resolving this one now.

Comment 12

[Axel Hecht]

mass verifying