From: Daniel Veillard (Daniel.Veillard@w3.org)
Date: Thu Oct 26 2000 - 08:57:06 EDT
On Thu, Oct 26, 2000 at 01:33:16AM +0200, TOM wrote:
>
> Hi,
> The attached patch fixes many bugs in XPath implementation:
First question:
- did you compile it ? Some added to xmlXPathNumberFunction()
is:
xmlChar* content = xmlNodeGetContent(ctxt->content->node);
clearly this is wrong, this should be
xmlChar* content = xmlNodeGetContent(ctxt->context->node);
and there is a couple of other obvious errors proving this
wasn't even compiled.
Please check the patches before sending them. Moreover,
test them for memory leaks by configuring --with-mem-debug
and running make tests if you can.
If you don't do it you put a lot more work on me, and it really
lower the level of trust I will give to patches. A compiler
can catch the obvious bugs and those are not a problem, but
sometime real bugs happen, and I'm not necessarily the best one
to find it if it comes in a contributed patch.
> - errors in function names (local-name() and namespace-uri(), not
> local-part() and namespace(); C function names now are
> xmlXPathLocalNameFunction() and xmlXPathNamespaceURIFunction())
minor, applied but now that those function names get exported
via xpathInternals.h future changes to just rename function will
not be accepted to not break binary compatibility.
> - some functions didn't allow optional arguments (xmlXPathLocalName(),
> xmlXPathNameFunction(), xmlXPathStringFunction(),
> xmlXPathNumberFunction(),
>
> - adds the sum() function
>
> - implements some TODO parts (xmlXPathIdFunction() with node-set
> argument, xmlXPathNormalizeFunction()
thanks
> - change xmlXPathFloorFunction(), xmlXPathCeilingFunction() and
> xmlXPathRoundFunction() to use floor() and ceil() if we have math.h
Okay, I hope it won't break on some strange architecture.
> - add the description of the -xptr option in testXPath's help message
thanks :-)
> - some corrections in help comments (xmlXPathStringEvalNumber() and
> xmlXPathEvalNumber() : the BUG was already fixed and the Number definition
> wasn't the one of the Recommandation)
yes the code was revamped (I initially coded it against a working draft).
thanks for finding this.
> - rename xmlXPathMINF to xmlXPathNINF (I think in xmlXPathPINF PINF
> means Positive INFinity, then it should be xmlXPathNINF for Negative
> INFinity)
yeah,
> There are still big bugs (I don't yet try to fix them) :
> - it is always assumed node-sets are ordered in document order, it
> isn't always the case (with the ancestor, ancestor-or-self, preceding and
> preceding-sibling [and parent] axes). We probably need an
> xmlXPathNodeSetGetFirst() function to get the node in the node-set that appears first in
> document order.
I have some node ordering test code in XPointer already, I will need
to backport it.
> - the same lack of ordering rigour leads
> "/EXAMPLE/chapter[last()]/preceding-sibling::chapter[1]" (should select /EXAMPLE/chapter[last() - 1])
> and "(/EXAMPLE/chapter[last()]/preceding-sibling::chapter)[1]" (should
> select /EXAMPLE/chapter[1]) to be equivalent (always selects
> /EXAMPLE/chapter[last() - 1]). We probably need ordering functions.
>
> - doesn't seem there is a namespace support for functions (last() is
> equivalent to foo:last() and bar:last()), it'll be necessary for
> proprietary XPath extension functions.
humm, do we really need namespace in that case, IMHO it should just be
string comparisons.
> - number() (xmlXPathNumberFunction() or xmlXPathStringEvalNumber())
> should be rewritten to handle "flip sign": number("-1") evaluates to NaN
> rather than -1.
<grin/>
> I didn't know what to change (it would be easier to modify
> xmlXPathStringEvalNumber()) so I did nothing.
yes that's probably the right place to handle this.
> - it seems impossible to pass node-sets (actually only relative paths
> or abbreviated absolute paths, and the root node) to functions:
> ptittom:~/libxml/devel$ ./testXPath --expr 'string(//title)'
> Error xpath.c:2345: Invalid type
> string(//title)
> ^
> Segfault
> ptittom:~/libxml/devel$ ./testXPath --expr 'string(title)'
> Error xpath.c:4292: Invalid expression
> number(title)
> ^
> Error xpath.c:4070: Invalid expression
> number(title)
> ^
> Object is empty (NULL)
> ptittom:~/libxml/devel$ ./testXPath --expr 'string(/)'
> Error xpath.c:4824: Invalid expression
> number(/)
> ^
> Segfault
gdb seems the only way to get this fixed . There is still a couple
of evaluation problems, right.
> - I don't understand why xmlXPathEval() (actually xmlXPathEvalExpr()
> called within xmlXPathEval()) must return a node-set.
> ptittom:~/libxml/devel$ ./testXPath --expr 'local-name(/EXAMPLE)'
> Object is a string : EXAMPLE
> ptittom:~/libxml/devel$ ./testXPath 'local-name(/EXAMPLE)'
> xmlXPathEval: evaluation failed to return a node set
> xmlXPathEval: 1 object left on the stack
> Object is empty (NULL)
yes that must be fixed.
> I hope forgetting nothing...
>
> Note: I haven't yet applied the "big patch" you mentionned, hope this
> won't break mine...
Seems it didn't. You should double check later once I have commited
your patches into CVS.
Daniel
-- Daniel.Veillard@w3.org | W3C, INRIA Rhone-Alpes | libxml Gnome XML toolkit Tel : +33 476 615 257 | 655, avenue de l'Europe | http://xmlsoft.org/ Fax : +33 476 615 207 | 38330 Montbonnot FRANCE | Rpmfind search site http://www.w3.org/People/all#veillard%40w3.org | http://rpmfind.net/ ---- Message from the list xml@rpmfind.net Archived at : http://xmlsoft.org/messages/ to unsubscribe: echo "unsubscribe xml" | mail majordomo@rpmfind.net
This archive was generated by hypermail 2b29 : Thu Oct 26 2000 - 09:43:46 EDT