Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Work around broken vsnprintf implementation in mingw. #993

Merged
merged 1 commit into from Nov 1, 2013

Conversation

larskanis
Copy link
Member

This fixes currently failing tests in XML::TestSaxEntityReference: https://gist.github.com/larskanis/7186885

It affects the currently released nokogiri-1.6.0-x86-mingw32.gem for both ruby-1.9 and ruby-2.0.

I found that the following build environments have this bug and verified that the attached patch works around it on all of them:
gcc (tdm-1) 4.5.2 - used for native builds on ruby-1.9
gcc (rubenvb-4.7.2-release) 4.7.2 - used for native build on ruby-2.0 (both x86 and x64)
i586-mingw32msvc-gcc (GCC) 4.2.1-sjlj (mingw32-2) - used to cross build for ruby-1.9
x86_64-w64-mingw32-gcc (rubenvb-4.7.2-release) 4.7.2 - used to cross build for ruby-2.0-x64
i686-w64-mingw32-gcc (rubenvb-4.7.2-release) 4.7.2 - used to cross build for ruby-2.0-x86

This issue is independent from #989, but also affects the x64-mingw32 version.

All nokogiri tests pass when the gem is built with any of the above environments and when using this PR together with #989. Verified with Ruby-x64 2.0.0-p0, Ruby-x86 2.0.0-p0 and Ruby-x86 1.9.3-p392 installed per RubyInstaller.

The buildin vasprintf() function is also used on Solaris, which I'm not able to test. But the given workaround should work on any platform that implements vsnprintf() according to C99.

This fixes failing tests in XML::TestSaxEntityReference.
@larskanis
Copy link
Member Author

The failed test cases in travis are not related to this pull request, but latest travis tests on master seems to fail generally. Locally I don't get these failures running on Ubuntu-12.04 version info: {"warnings"=>[], "nokogiri"=>"1.6.0", "ruby"=>{"version"=>"1.9.3", "platform"=>"java", "description"=>"jruby 1.7.6 (1.9.3p392) 2013-10-22 6004147 on OpenJDK 64-Bit Server VM 1.6.0_27-b27 [linux-amd64]", "engine"=>"jruby", "jruby"=>"1.7.6"}, "xerces"=>"Xerces-J 2.9.0", "nekohtml"=>"NekoHTML 1.9.12"}

@knu
Copy link
Member

knu commented Nov 1, 2013

Is the bug filed in the upstream? If the bug is likely to affect other programs than Nokogiri, it should be submitted.

@knu
Copy link
Member

knu commented Nov 1, 2013

Out of curiousity, is the root of the problem a NULL pointer, 0 in size or both?

@knu
Copy link
Member

knu commented Nov 1, 2013

@larskanis I know, the failure is related to a bug in JRuby Nokogiri and irrelevant to this pull request.

@larskanis
Copy link
Member Author

The 0 in size is the problem. I found this patch in ffmpeg. RubyInstaller is using mingw with __MINGW32_MAJOR_VERSION = 2. I'll try to find a bug report upstream or the relevant change in the mingw sources.

Maybe it is better to use _scprintf to determine the required size instead, but I didn't test this on all relevant mingw platforms, right now.

knu added a commit that referenced this pull request Nov 1, 2013
Work around broken vsnprintf implementation in mingw.
@knu knu merged commit 1980c96 into sparklemotion:master Nov 1, 2013
@knu
Copy link
Member

knu commented Nov 1, 2013

OK, thanks for the follow-up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants