remove calls to vasprintf

[flavorjones]

What problem is this PR intended to solve?

#2481 proposed cleaning up our local implementation of vasprintf and using it on all platforms; however this PR takes an alternative approach of removing calls to vasprintf in favor of using rb_vsprintf since we need to create Ruby String objects from all the errors being handled this way.

Some nice side effects:

• TruffleRuby should like this better, since Sulong was missing functionality around varargs
• we no longer have to do platform-specific detection of vasprintf at compile time

Note also that this PR is based on #2480, so that should be considered before this one.

Have you included adequate test coverage?

Yes! Some of the error handling code did not have good, or any, coverage. Test coverage has been added in this PR.

Does this change affect the behavior of either the C or the Java implementations?

No behavioral changes are being introduced.

cc @Garfield96 @stevecheckoway

[larskanis]

Thanks for asking! This looks all great! No RB_GC_GUARD necessary.

There's a single remaining stdc free() call:

nokogiri/ext/nokogiri/gumbo.c

Line 276 in 1b7f259

free(msg);

It should be gumbo_free() instead. Or even better gumbo_alloc() and siblings could be converted to ruby_xmalloc equally.

[flavorjones]

@larskanis I saw that free call, but left it for now because gumbo_alloc and gumbo_free aren't public.

I agree it would be nice to add a feature to libgumbo to allow us to set the alloc/free functions (like xmlMemSetup does for libxml2), so I've created #2490 to drive that discussion.

[stevecheckoway]

At the moment, Gumbo isn't tied to ruby in any way, I think it makes sense to keep it that way, absent a compelling reason.

Gumbo originally had a way of supporting other allocators, but it made the code worse and was removed. It makes sense to me to keep Gumbo's "public" API returning memory from the standard allocator. I don't think any of the utility functions in util.h should be part of Gumbo's public API.

I'd be in favor of removing gumbo_free() entirely and just replacing it with calls to free().

[flavorjones]

Ah, OK, @stevecheckoway I just saw your update and will defer to your decision if you feel strongly. Go ahead and close #2490 if you wish.

[flavorjones]

I've rebased after merging #2480 and so this changeset should be more readable now.

[flavorjones]

I also kicked off the truffle CI jobs on this branch because I think this PR may break some of the patches being applied in https://github.com/oracle/truffleruby/blob/master/lib/truffle/truffle/patches/nokogiri_patches.rb

https://github.com/sparklemotion/nokogiri/actions/runs/2082546265

cc @eregon @aardvark179

[eregon]

@flavorjones Those runs failed, any idea of the issue?
For the first 2 I can't find the error, maybe Expected '| ' but got ?

For the 3rd job it's:

 /__w/nokogiri/nokogiri/ext/nokogiri/xml_xpath_context.c:301:63: warning: function 'xpath_generic_exception_handler' declared 'noreturn' should not return [-Winvalid-noreturn]
va_list args; rb_raise(rb_eRuntimeError, "%s", "Exception:"); return;
                                                              ^
/__w/nokogiri/nokogiri/ext/nokogiri/xml_xpath_context.c:344:27: error: use of undeclared identifier 'errors'
xmlSetStructuredErrorFunc(errors, Nokogiri_error_array_pusher);
                          ^
1 warning and 1 error generated.

It fails the same way on master (https://github.com/sparklemotion/nokogiri/runs/5800516006?check_suite_focus=true)

[flavorjones]

@eregon some of the TR patches are no longer applying, let's discuss at #2491