From af4c9fa086aee5b41d7c5864bff34e01223ecd4f Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Fri, 27 Dec 2013 09:45:49 +0000
Subject: [PATCH 017/118] 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.
---
 generator/bindtests.ml           |  2 +-
 generator/ruby.ml                | 92 ++++++++++++++++++++++++++++++++--------
 ruby/tests/tc_010_load.rb        |  2 +-
 ruby/tests/tc_050_lvcreate.rb    |  2 +-
 ruby/tests/tc_060_optargs.rb     |  2 +-
 ruby/tests/tc_400_events.rb      |  2 +-
 ruby/tests/tc_820_rhbz1046509.rb | 42 ++++++++++++++++++
 ruby/tests/tc_rhbz507346.rb      |  2 +-
 ruby/tests/tc_rhbz664558c6.rb    |  2 +-
 9 files changed, 124 insertions(+), 24 deletions(-)
 create mode 100644 ruby/tests/tc_820_rhbz1046509.rb

diff --git a/generator/bindtests.ml b/generator/bindtests.ml
index 55c39cb..86edd4c 100644
--- a/generator/bindtests.ml
+++ b/generator/bindtests.ml
@@ -451,7 +451,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 8a54073..899b50b 100644
--- a/generator/ruby.ml
+++ b/generator/ruby.ml
@@ -40,6 +40,7 @@ let rec generate_ruby_c () =
 #include <stdio.h>
 #include <stdlib.h>
 #include <stdint.h>
+#include <assert.h>
 
 #include <ruby.h>
 
@@ -130,34 +131,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)
@@ -166,9 +222,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);
 }
 
@@ -695,10 +748,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);
@@ -707,6 +760,11 @@ Init__guestfs (void)
   rb_define_method (c_guestfs, \"user_cancel\",
                     ruby_user_cancel, 0);
 
+  /* 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/tests/tc_010_load.rb b/ruby/tests/tc_010_load.rb
index 80c1e03..3ad71f2 100644
--- a/ruby/tests/tc_010_load.rb
+++ b/ruby/tests/tc_010_load.rb
@@ -22,7 +22,7 @@ require 'guestfs'
 
 class TestLoad < Test::Unit::TestCase
   def test_load
-    g = Guestfs::create()
+    g = Guestfs::Guestfs.new()
     assert_not_nil (g)
   end
 end
diff --git a/ruby/tests/tc_050_lvcreate.rb b/ruby/tests/tc_050_lvcreate.rb
index 4e9c957..d03544f 100644
--- a/ruby/tests/tc_050_lvcreate.rb
+++ b/ruby/tests/tc_050_lvcreate.rb
@@ -22,7 +22,7 @@ require 'guestfs'
 
 class TestLoad < Test::Unit::TestCase
   def test_lvcreate
-    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/tests/tc_060_optargs.rb b/ruby/tests/tc_060_optargs.rb
index 4506466..e28f944 100644
--- a/ruby/tests/tc_060_optargs.rb
+++ b/ruby/tests/tc_060_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/tests/tc_400_events.rb b/ruby/tests/tc_400_events.rb
index 4e04570..e27e215 100644
--- a/ruby/tests/tc_400_events.rb
+++ b/ruby/tests/tc_400_events.rb
@@ -22,7 +22,7 @@ require 'guestfs'
 
 class TestLoad < Test::Unit::TestCase
   def test_events
-    g = Guestfs::create()
+    g = Guestfs::Guestfs.new()
 
     log = Proc.new {| event, event_handle, buf, array |
       if event == Guestfs::EVENT_APPLIANCE
diff --git a/ruby/tests/tc_820_rhbz1046509.rb b/ruby/tests/tc_820_rhbz1046509.rb
new file mode 100644
index 0000000..978decd
--- /dev/null
+++ b/ruby/tests/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
diff --git a/ruby/tests/tc_rhbz507346.rb b/ruby/tests/tc_rhbz507346.rb
index 6082201..7b8d526 100644
--- a/ruby/tests/tc_rhbz507346.rb
+++ b/ruby/tests/tc_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/tests/tc_rhbz664558c6.rb b/ruby/tests/tc_rhbz664558c6.rb
index 290deac..5eb373e 100644
--- a/ruby/tests/tc_rhbz664558c6.rb
+++ b/ruby/tests/tc_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 |
-- 
2.7.4

