Fixes on string XPath functions

[waj]

Please VERIFY and let's discuss about this. I think the fix is correct, but just in case...

The NOKOGIRI_STR_NEW2 it's duplicating the string and the xpath->stringval is not released because xmlXPathFreeNodeSetList is used at the end of the evaluate function, and I checked the libxml source and it just releases the xmlXPathObject struct.

The second commit fixes a bug for custom XPath functions that returns strings. Currently the string is being wrapped but it might be released by the GC so at the end only garbage is returned.

[ender672]

Both fixes look great.

I noticed in include/libxml/xpathInternals.h:

#define xmlXPathReturnString(ctxt, str)                 \
    valuePush((ctxt), xmlXPathWrapString(str))

And in Nokogiri, before your patch:

  xmlXPathReturnString(
      ctx,
      (xmlChar *)xmlXPathWrapCString(StringValuePtr(result))
  );

Which looks like was resulting in the string being wrapped twice. Your patch looks much better.

I also ran a test to make sure that the allocation by xmlCharStrdup() gets freed by libxml2 (though I didn't actually dig through libxml2 to confirm exactly how it happens).

I feel warm and fuzzy about these patches. Are you comfortable if I merge this? You mentioned you wanted some discussion first ...

[waj]

I just wanted someone else to check what I did because I don't have much experience with libxml internals.

I added another commit because I forgot the namespace prefix when Nokogiri.uses_libxml? is false.

BTW, how can I run the tests without libxml so I can test that branch of the if's?

[ender672]

use rvm or rbenv to switch to jruby

$ rbenv shell jruby-1.6.5 
$ bundle exec rake clobber && bundle exec rake

Expect some unit test errors in jruby. You'll want to run the tests against master and compare the error count.

[waj]

I cannot get it working:

install -c tmp/java/nokogiri/nokogiri.jar lib/nokogiri/nokogiri.jar
/Users/waj/Sandbox/nokogiri/.bundle/jruby/1.8/bin/racc -l -o lib/nokogiri/css/parser.rb lib/nokogiri/css/parser.y
ArgumentError: rb_iterate only supported with rb_each and an Array
     send at org/jruby/RubyKernel.java:2105
  yyparse at /Users/waj/Sandbox/nokogiri/.bundle/jruby/1.8/gems/racc-1.4.7/lib/racc/parser.rb:152
    parse at /Users/waj/Sandbox/nokogiri/.bundle/jruby/1.8/gems/racc-1.4.7/lib/racc/grammarfileparser.rb:184
     main at /Users/waj/Sandbox/nokogiri/.bundle/jruby/1.8/gems/racc-1.4.7/bin/racc:143
  section at /Users/waj/Sandbox/nokogiri/.bundle/jruby/1.8/gems/racc-1.4.7/bin/racc:277
     main at /Users/waj/Sandbox/nokogiri/.bundle/jruby/1.8/gems/racc-1.4.7/bin/racc:141
   (root) at /Users/waj/Sandbox/nokogiri/.bundle/jruby/1.8/gems/racc-1.4.7/bin/racc:308
     load at org/jruby/RubyKernel.java:1063
   (root) at /Users/waj/Sandbox/nokogiri/.bundle/jruby/1.8/bin/racc:19
rake aborted!

[ender672]

Nokogiri invokes racc as part of the build process. When in JRuby, racc should be using the pure-ruby parser, but it appears to be invoking the C parser.

You'll have to figure out what bundler and rvm/rbenv are doing.

[ender672]

BTW, I tested your patches against JRuby and the tests pass.

[waj]

Great! please, merge it then if you agree.

I'm still fighting with the JRuby environment, but I'm not going to let it win ;-)