220703 - txExprParser::createExpr should either return NS_FAILED or an Expr

Status: RESOLVED FIXED

(Core :: XSLT, defect)

Description

[Axel Hecht]

the nsresult returned by createExpr should be either a error value or a 
non-null expr, no mixture.
This means that fcp needs to be handled outside of createExpr in 
txStylesheetCompileHandlers.cpp, getExprAttr.

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Any progress on this? This is a pretty serious regression since crashers were
introduced.

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

err.. pressed submit prematurly. 1.6a is comming up in a couple of days and it'd
suck to have that released with known xslt regressions.

Comment 3

[Axel Hecht]

Attachment: fixing the leak

Comment 4

[Axel Hecht]

I don't see a crash anywhere. We never return NS_OK for a null expression. 
This code leaked, though.

Note, I'm not doing fcp. I don't expect to get reviews on anything I do, so I'm
not doing anything.
Rationale for my opinion:
fcp is pretty useless in its own right, xslt1.1 is dead and xslt2 is just too
far away to be actually of any use in a 1.0 processor.
I'd only do a single expression, hooked up in getExprAttr to get fcp done, which
conflicts with the language of the fcp spec as far as function argument counts 
go.
That says "should only error if the function is called". Of course the only way
for that not to happen in a expr evaluation would be a 
(function-available(foo) and foo('way', 'too', 'many', 'arguments')).
Of course that's pretty useless in most cases, as that works only for boolean
function call results *and* under the assumption that false is a good default
value. It's just silly language stating something completely irrelevant.
Not gonna go on my time.

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

it returns NS_OK and a null-expression for the empty string so the current patch
only fixes part of this bug.

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 133755 [details] [diff] [review]
fixing the leak

upps, that should be |*aExpr| and not |aExpr|

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: patch to fix

The change to the lexer makes the lexer treat "" as " ", which then the parser
catches as a parse-error

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

checked in, thanks for reviews