From d299014ef6b766c23db2f2a11fb21d5df7123e51 Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" Date: Fri, 27 Dec 2013 09:45:49 +0000 Subject: [PATCH] ruby: Fix .new method (RHBZ#1046509). The .new method was unintentionally broken in commit 9466060201600db47016133d80af22eb38091a49. This fixes the .new method and allows it to be called with multiple parameters, so you can use: Guestfs::Guestfs.new Guestfs::Guestfs.new() Guestfs::Guestfs.new(:close_on_exit => false) etc. For backwards compatibility, Guestfs::create may still be used. This commit also adds regression tests: - Use .new method in regular tests. (Because this was not done before, we didn't catch the breakage.) - Test that ::create still works. - Test that args can be passed to .new method. (cherry picked from commit ee4ce2a0298d012bd8c500c35dc50e1f53e88c8b) --- generator/bindtests.ml | 2 +- generator/ruby.ml | 92 +++++++++++++++++++++++++++++++------- ruby/t/tc_020_create.rb | 2 +- ruby/t/tc_030_create_flags.rb | 29 ++++++++++++ ruby/t/tc_040_create_multiple.rb | 32 +++++++++++++ ruby/t/tc_050_handle_properties.rb | 36 +++++++++++++++ ruby/t/tc_060_explicit_close.rb | 29 ++++++++++++ ruby/t/tc_070_optargs.rb | 2 +- ruby/t/tc_100_launch.rb | 2 +- ruby/t/tc_410_close_event.rb | 2 +- ruby/t/tc_420_log_messages.rb | 2 +- ruby/t/tc_800_rhbz507346.rb | 2 +- ruby/t/tc_810_rhbz664558c6.rb | 2 +- ruby/t/tc_820_rhbz1046509.rb | 42 +++++++++++++++++ 14 files changed, 251 insertions(+), 25 deletions(-) create mode 100644 ruby/t/tc_030_create_flags.rb create mode 100644 ruby/t/tc_040_create_multiple.rb create mode 100644 ruby/t/tc_050_handle_properties.rb create mode 100644 ruby/t/tc_060_explicit_close.rb create mode 100644 ruby/t/tc_820_rhbz1046509.rb diff --git a/generator/bindtests.ml b/generator/bindtests.ml index 55b6a81..e18b9be 100644 --- a/generator/bindtests.ml +++ b/generator/bindtests.ml @@ -452,7 +452,7 @@ and generate_ruby_bindtests () = pr "\ require 'guestfs' -g = Guestfs::create() +g = Guestfs::Guestfs.new() "; let mkargs args optargs = diff --git a/generator/ruby.ml b/generator/ruby.ml index 3a7d05d..eb176ab 100644 --- a/generator/ruby.ml +++ b/generator/ruby.ml @@ -42,6 +42,7 @@ let rec generate_ruby_c () = #include #include #include +#include #pragma GCC diagnostic push #pragma GCC diagnostic ignored \"-Wstrict-prototypes\" @@ -135,34 +136,89 @@ ruby_guestfs_free (void *gvp) } } +/* This is the ruby internal alloc function for the class. We do nothing + * here except allocate an object containing a NULL guestfs handle. + * Note we cannot call guestfs_create here because we need the extra + * parameters, which ruby passes via the initialize method (see next + * function). + */ +static VALUE +ruby_guestfs_alloc (VALUE klass) +{ + guestfs_h *g = NULL; + + /* Wrap it, and make sure the close function is called when the + * handle goes away. + */ + return Data_Wrap_Struct (c_guestfs, NULL, ruby_guestfs_free, g); +} + +static unsigned +parse_flags (int argc, VALUE *argv) +{ + volatile VALUE optargsv; + unsigned flags = 0; + volatile VALUE v; + + optargsv = argc == 1 ? argv[0] : rb_hash_new (); + Check_Type (optargsv, T_HASH); + + v = rb_hash_lookup (optargsv, ID2SYM (rb_intern (\"environment\"))); + if (v != Qnil && !RTEST (v)) + flags |= GUESTFS_CREATE_NO_ENVIRONMENT; + v = rb_hash_lookup (optargsv, ID2SYM (rb_intern (\"close_on_exit\"))); + if (v != Qnil && !RTEST (v)) + flags |= GUESTFS_CREATE_NO_CLOSE_ON_EXIT; + + return flags; +} + /* * call-seq: * Guestfs::Guestfs.new([{:environment => false, :close_on_exit => false}]) -> Guestfs::Guestfs * * Call - * +guestfs_create+[http://libguestfs.org/guestfs.3.html#guestfs_create] + * +guestfs_create_flags+[http://libguestfs.org/guestfs.3.html#guestfs_create_flags] * to create a new libguestfs handle. The handle is represented in * Ruby as an instance of the Guestfs::Guestfs class. */ static VALUE -ruby_guestfs_create (int argc, VALUE *argv, VALUE m) +ruby_guestfs_initialize (int argc, VALUE *argv, VALUE m) { guestfs_h *g; + unsigned flags; if (argc > 1) rb_raise (rb_eArgError, \"expecting 0 or 1 arguments\"); - volatile VALUE optargsv = argc == 1 ? argv[0] : rb_hash_new (); - Check_Type (optargsv, T_HASH); + /* Should have been set to NULL by prior call to alloc function. */ + assert (DATA_PTR (m) == NULL); - unsigned flags = 0; - volatile VALUE v; - v = rb_hash_lookup (optargsv, ID2SYM (rb_intern (\"environment\"))); - if (v != Qnil && !RTEST (v)) - flags |= GUESTFS_CREATE_NO_ENVIRONMENT; - v = rb_hash_lookup (optargsv, ID2SYM (rb_intern (\"close_on_exit\"))); - if (v != Qnil && !RTEST (v)) - flags |= GUESTFS_CREATE_NO_CLOSE_ON_EXIT; + flags = parse_flags (argc, argv); + + g = guestfs_create_flags (flags); + if (!g) + rb_raise (e_Error, \"failed to create guestfs handle\"); + + DATA_PTR (m) = g; + + /* Don't print error messages to stderr by default. */ + guestfs_set_error_handler (g, NULL, NULL); + + return m; +} + +/* For backwards compatibility. */ +static VALUE +ruby_guestfs_create (int argc, VALUE *argv, VALUE module) +{ + guestfs_h *g; + unsigned flags; + + if (argc > 1) + rb_raise (rb_eArgError, \"expecting 0 or 1 arguments\"); + + flags = parse_flags (argc, argv); g = guestfs_create_flags (flags); if (!g) @@ -171,9 +227,6 @@ ruby_guestfs_create (int argc, VALUE *argv, VALUE m) /* Don't print error messages to stderr by default. */ guestfs_set_error_handler (g, NULL, NULL); - /* Wrap it, and make sure the close function is called when the - * handle goes away. - */ return Data_Wrap_Struct (c_guestfs, NULL, ruby_guestfs_free, g); } @@ -707,10 +760,10 @@ Init__guestfs (void) #ifndef HAVE_TYPE_RB_ALLOC_FUNC_T #define rb_alloc_func_t void* #endif - rb_define_alloc_func (c_guestfs, (rb_alloc_func_t) ruby_guestfs_create); + rb_define_alloc_func (c_guestfs, (rb_alloc_func_t) ruby_guestfs_alloc); #endif - rb_define_module_function (m_guestfs, \"create\", ruby_guestfs_create, -1); + rb_define_method (c_guestfs, \"initialize\", ruby_guestfs_initialize, -1); rb_define_method (c_guestfs, \"close\", ruby_guestfs_close, 0); rb_define_method (c_guestfs, \"set_event_callback\", ruby_set_event_callback, 2); @@ -719,6 +772,11 @@ Init__guestfs (void) rb_define_module_function (m_guestfs, \"event_to_string\", ruby_event_to_string, 1); + /* For backwards compatibility with older code, define a ::create + * module function. + */ + rb_define_module_function (m_guestfs, \"create\", ruby_guestfs_create, -1); + "; (* Constants. *) diff --git a/ruby/t/tc_020_create.rb b/ruby/t/tc_020_create.rb index 2b57ba1..4a04e97 100644 --- a/ruby/t/tc_020_create.rb +++ b/ruby/t/tc_020_create.rb @@ -22,7 +22,7 @@ require 'guestfs' class TestLoad < Test::Unit::TestCase def test_create - g = Guestfs::create() + g = Guestfs::Guestfs.new() assert_not_nil (g) end end diff --git a/ruby/t/tc_030_create_flags.rb b/ruby/t/tc_030_create_flags.rb new file mode 100644 index 0000000..7ec2ce9 --- /dev/null +++ b/ruby/t/tc_030_create_flags.rb @@ -0,0 +1,29 @@ +# libguestfs Ruby bindings -*- ruby -*- +# Copyright (C) 2013 Red Hat Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +require 'test/unit' +$:.unshift(File::join(File::dirname(__FILE__), "..", "lib")) +$:.unshift(File::join(File::dirname(__FILE__), "..", "ext", "guestfs")) +require 'guestfs' + +class TestLoad < Test::Unit::TestCase + def test_create_flags + g = Guestfs::Guestfs.new(:environment => false, :close_on_exit => true) + assert_not_nil (g) + g.parse_environment() + end +end diff --git a/ruby/t/tc_040_create_multiple.rb b/ruby/t/tc_040_create_multiple.rb new file mode 100644 index 0000000..339b699 --- /dev/null +++ b/ruby/t/tc_040_create_multiple.rb @@ -0,0 +1,32 @@ +# libguestfs Ruby bindings -*- ruby -*- +# Copyright (C) 2013 Red Hat Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +require 'test/unit' +$:.unshift(File::join(File::dirname(__FILE__), "..", "lib")) +$:.unshift(File::join(File::dirname(__FILE__), "..", "ext", "guestfs")) +require 'guestfs' + +class TestLoad < Test::Unit::TestCase + def test_create_multiple + g1 = Guestfs::Guestfs.new() + g2 = Guestfs::Guestfs.new() + g3 = Guestfs::Guestfs.new() + assert_not_nil (g1) + assert_not_nil (g2) + assert_not_nil (g3) + end +end diff --git a/ruby/t/tc_050_handle_properties.rb b/ruby/t/tc_050_handle_properties.rb new file mode 100644 index 0000000..cf4a7a7 --- /dev/null +++ b/ruby/t/tc_050_handle_properties.rb @@ -0,0 +1,36 @@ +# libguestfs Ruby bindings -*- ruby -*- +# Copyright (C) 2013 Red Hat Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +require 'test/unit' +$:.unshift(File::join(File::dirname(__FILE__), "..", "lib")) +$:.unshift(File::join(File::dirname(__FILE__), "..", "ext", "guestfs")) +require 'guestfs' + +class TestLoad < Test::Unit::TestCase + def test_handle_properties + g = Guestfs::Guestfs.new() + assert_not_nil (g) + v = g.get_verbose() + g.set_verbose(v) + v = g.get_trace() + g.set_trace(v) + v = g.get_memsize() + g.set_memsize(v) + v = g.get_path() + g.set_path(v) + end +end diff --git a/ruby/t/tc_060_explicit_close.rb b/ruby/t/tc_060_explicit_close.rb new file mode 100644 index 0000000..74456e4 --- /dev/null +++ b/ruby/t/tc_060_explicit_close.rb @@ -0,0 +1,29 @@ +# libguestfs Ruby bindings -*- ruby -*- +# Copyright (C) 2013 Red Hat Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +require 'test/unit' +$:.unshift(File::join(File::dirname(__FILE__), "..", "lib")) +$:.unshift(File::join(File::dirname(__FILE__), "..", "ext", "guestfs")) +require 'guestfs' + +class TestLoad < Test::Unit::TestCase + def test_explicit_close + g = Guestfs::Guestfs.new() + assert_not_nil (g) + g.close() + end +end diff --git a/ruby/t/tc_070_optargs.rb b/ruby/t/tc_070_optargs.rb index 4506466..e28f944 100644 --- a/ruby/t/tc_070_optargs.rb +++ b/ruby/t/tc_070_optargs.rb @@ -22,7 +22,7 @@ require 'guestfs' class TestLoad < Test::Unit::TestCase def test_optargs - g = Guestfs::create() + g = Guestfs::Guestfs.new() g.add_drive("/dev/null", {}) g.add_drive("/dev/null", :readonly => 1) diff --git a/ruby/t/tc_100_launch.rb b/ruby/t/tc_100_launch.rb index 4bd8187..f318ae8 100644 --- a/ruby/t/tc_100_launch.rb +++ b/ruby/t/tc_100_launch.rb @@ -22,7 +22,7 @@ require 'guestfs' class TestLoad < Test::Unit::TestCase def test_launch - g = Guestfs::create() + g = Guestfs::Guestfs.new() File.open("test.img", "w") { |f| f.seek(500*1024*1024); f.write("\0") diff --git a/ruby/t/tc_410_close_event.rb b/ruby/t/tc_410_close_event.rb index ebf4bbc..d7e53d4 100644 --- a/ruby/t/tc_410_close_event.rb +++ b/ruby/t/tc_410_close_event.rb @@ -22,7 +22,7 @@ require 'guestfs' class TestLoad < Test::Unit::TestCase def test_events - g = Guestfs::create() + g = Guestfs::Guestfs.new() close_invoked = 0 close = Proc.new {| event, event_handle, buf, array | diff --git a/ruby/t/tc_420_log_messages.rb b/ruby/t/tc_420_log_messages.rb index d23a0fb..0c3bd4d 100644 --- a/ruby/t/tc_420_log_messages.rb +++ b/ruby/t/tc_420_log_messages.rb @@ -22,7 +22,7 @@ require 'guestfs' class TestLoad < Test::Unit::TestCase def test_events - g = Guestfs::create() + g = Guestfs::Guestfs.new() log_invoked = 0 log = Proc.new {| event, event_handle, buf, array | diff --git a/ruby/t/tc_800_rhbz507346.rb b/ruby/t/tc_800_rhbz507346.rb index 6082201..7b8d526 100644 --- a/ruby/t/tc_800_rhbz507346.rb +++ b/ruby/t/tc_800_rhbz507346.rb @@ -22,7 +22,7 @@ require 'guestfs' class TestLoad < Test::Unit::TestCase def test_rhbz507346 - g = Guestfs::create() + g = Guestfs::Guestfs.new() File.open("test.img", "w") { |f| f.seek(10*1024*1024); f.write("\0") diff --git a/ruby/t/tc_810_rhbz664558c6.rb b/ruby/t/tc_810_rhbz664558c6.rb index 290deac..5eb373e 100644 --- a/ruby/t/tc_810_rhbz664558c6.rb +++ b/ruby/t/tc_810_rhbz664558c6.rb @@ -26,7 +26,7 @@ require 'guestfs' class TestLoad < Test::Unit::TestCase def test_rhbz664558c6 - g = Guestfs::create() + g = Guestfs::Guestfs.new() close_invoked = 0 close = Proc.new {| event, event_handle, buf, array | diff --git a/ruby/t/tc_820_rhbz1046509.rb b/ruby/t/tc_820_rhbz1046509.rb new file mode 100644 index 0000000..978decd --- /dev/null +++ b/ruby/t/tc_820_rhbz1046509.rb @@ -0,0 +1,42 @@ +# libguestfs Ruby bindings -*- ruby -*- +# Copyright (C) 2013 Red Hat Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +# Test that we don't break the old ::create module function while +# fixing https://bugzilla.redhat.com/show_bug.cgi?id=1046509 + +require 'test/unit' +$:.unshift(File::join(File::dirname(__FILE__), "..", "lib")) +$:.unshift(File::join(File::dirname(__FILE__), "..", "ext", "guestfs")) +require 'guestfs' + +class TestLoad < Test::Unit::TestCase + def _handleok(g) + g.add_drive("/dev/null") + g.close() + end + + def test_rhbz1046509 + g = Guestfs::create() + _handleok(g) + + g = Guestfs::create(:close_on_exit => true) + _handleok(g) + + g = Guestfs::create(:close_on_exit => true, :environment => true) + _handleok(g) + end +end -- 1.8.3.1