swig/swig308-Ruby-opaque-pointer...

216 lines
6.2 KiB
Diff

From 763827c2e1fffd34e871c66d5d4c9e492c1cecec Mon Sep 17 00:00:00 2001
From: William S Fulton <wsf@fultondesigns.co.uk>
Date: Tue, 24 May 2016 07:33:25 +0100
Subject: [PATCH] Ruby opaque pointer handling regression fix
This bug was introduced in swig-3.0.8 in #146252 adding shared_ptr
support. An ObjectPreviouslyDeleted error was incorrectly thrown
when the pointer was used as a parameter after being set to zero
via a call to 'DATA_PTR(self) = 0'.
It isn't clear to me which approach is better in this corner case,
so I've gone for backwards compatibility and restored the old behaviour.
Closes #602
---
CHANGES.current | 6 ++
Examples/test-suite/ruby/Makefile.in | 3 +-
.../test-suite/ruby/ruby_manual_proxy_runme.rb | 49 ++++++++++++++++
Examples/test-suite/ruby_manual_proxy.i | 66 ++++++++++++++++++++++
Lib/ruby/rubyrun.swg | 12 ++--
5 files changed, 131 insertions(+), 5 deletions(-)
create mode 100644 Examples/test-suite/ruby/ruby_manual_proxy_runme.rb
create mode 100644 Examples/test-suite/ruby_manual_proxy.i
#diff --git a/CHANGES.current b/CHANGES.current
#index 6e347a7..f754e96 100644
#--- a/CHANGES.current
#+++ b/CHANGES.current
#@@ -5,6 +5,12 @@ See the RELEASENOTES file for a summary of changes in each release.
# Version 3.0.9 (in progress)
# ===========================
#
#+2016-05-23: wsfulton
#+ [Ruby] Fix #602 - Error handling regression of opaque pointers introduced
#+ in swig-3.0.8 when C functions explicitly reset a pointer using 'DATA_PTR(self) = 0'.
#+ An ObjectPreviouslyDeleted error was incorrectly thrown when the pointer was used
#+ as a parameter.
#+
# 2016-05-17: tamuratak
# [Ruby] Patch #651 - Correct overloaded function error message when function is
# using %newobject.
diff --git a/Examples/test-suite/ruby/Makefile.in b/Examples/test-suite/ruby/Makefile.in
index 847c959..d94ac70 100644
--- a/Examples/test-suite/ruby/Makefile.in
+++ b/Examples/test-suite/ruby/Makefile.in
@@ -32,7 +32,8 @@ CPP_TEST_CASES = \
C_TEST_CASES += \
li_cdata \
- li_cstring
+ li_cstring \
+ ruby_manual_proxy \
include $(srcdir)/../common.mk
diff --git a/Examples/test-suite/ruby/ruby_manual_proxy_runme.rb b/Examples/test-suite/ruby/ruby_manual_proxy_runme.rb
new file mode 100644
index 0000000..c1cee2d
--- /dev/null
+++ b/Examples/test-suite/ruby/ruby_manual_proxy_runme.rb
@@ -0,0 +1,49 @@
+#!/usr/bin/env ruby
+#
+# The Subversion bindings use this manually written proxy class approach
+# to the Ruby bindings. Note that in C the struct svn_fs_t is an
+# opaque pointer and the Ruby FileSystem proxy class is hand written around it.
+# This testcase tests this and the C close function and subsequent error
+# handling.
+
+require 'swig_assert'
+require 'ruby_manual_proxy'
+
+module Svn
+ module Fs
+ module_function
+ def create(path)
+ f = Ruby_manual_proxy::svn_fs_create(path)
+ return f
+ end
+
+ FileSystem = SWIG::TYPE_p_svn_fs_t
+ class FileSystem
+ class << self
+ def create(*args)
+ Fs.create(*args)
+ end
+ end
+ def path
+ Ruby_manual_proxy::svn_fs_path(self)
+ end
+ end
+ end
+end
+
+f = Svn::Fs::FileSystem.create("/tmp/myfile")
+path = f.path
+f.close
+begin
+ # regression in swig-3.0.8 meant ObjectPreviouslyDeleted error was thrown instead
+ path = f.path
+ raise RuntimeError.new("IOError (1) not thrown")
+rescue IOError
+end
+
+file = nil
+begin
+ path = Ruby_manual_proxy::svn_fs_path(file)
+ raise RuntimeError.new("IOError (2) not thrown")
+rescue IOError
+end
diff --git a/Examples/test-suite/ruby_manual_proxy.i b/Examples/test-suite/ruby_manual_proxy.i
new file mode 100644
index 0000000..2cb154e
--- /dev/null
+++ b/Examples/test-suite/ruby_manual_proxy.i
@@ -0,0 +1,66 @@
+%module ruby_manual_proxy
+
+
+%typemap(in, numinputs=0) SWIGTYPE ** ($*1_ltype temp) "$1 = &temp;";
+
+%typemap(argout) SWIGTYPE **OUTPARAM {
+ $result = SWIG_Ruby_AppendOutput($result, SWIG_NewPointerObj(*$1, $*1_descriptor, 0));
+}
+
+%apply SWIGTYPE **OUTPARAM {
+ svn_fs_t **
+};
+
+%typemap(check) svn_fs_t * {
+ if (!$1) {
+ svn_swig_rb_raise_svn_fs_already_close();
+ }
+}
+
+%{
+typedef struct svn_fs_t {
+ char path[256];
+} svn_fs_t;
+
+void svn_fs_create(svn_fs_t **fs_p, const char *path) {
+ svn_fs_t *fs = (svn_fs_t *)malloc(sizeof(svn_fs_t));
+ strncpy(fs->path, path, 256);
+ *fs_p = fs;
+}
+const char *svn_fs_path(svn_fs_t *fs) {
+ return fs->path;
+}
+%}
+
+typedef struct svn_fs_t svn_fs_t;
+void svn_fs_create(svn_fs_t **fs_p, const char *path);
+const char *svn_fs_path(svn_fs_t *fs);
+
+%{
+static void svn_swig_rb_raise_svn_fs_already_close(void) {
+ rb_raise(rb_eIOError, "already closed");
+}
+
+static VALUE svn_fs_swig_rb_close(VALUE self) {
+ if (!DATA_PTR(self)) {
+ svn_swig_rb_raise_svn_fs_already_close();
+ }
+
+ DATA_PTR(self) = NULL;
+
+ return Qnil;
+}
+
+static VALUE svn_fs_swig_rb_closed(VALUE self) {
+ return DATA_PTR(self) ? Qfalse : Qtrue;
+}
+%}
+
+%insert("init") %{
+ {
+ VALUE cSvnfs;
+ cSvnfs = rb_const_get(_mSWIG, rb_intern("TYPE_p_svn_fs_t"));
+ rb_define_method(cSvnfs, "close",
+ VALUEFUNC(svn_fs_swig_rb_close), 0);
+ }
+%}
diff --git a/Lib/ruby/rubyrun.swg b/Lib/ruby/rubyrun.swg
index e18208f..249494a 100644
--- a/Lib/ruby/rubyrun.swg
+++ b/Lib/ruby/rubyrun.swg
@@ -305,6 +305,14 @@ SWIG_Ruby_ConvertPtrAndOwn(VALUE obj, void **ptr, swig_type_info *ty, int flags,
/* Do type-checking if type info was provided */
if (ty) {
+ if (ty->clientdata) {
+ if (rb_obj_is_kind_of(obj, ((swig_class *) (ty->clientdata))->klass)) {
+ if (vptr == 0) {
+ /* The object has already been deleted */
+ return SWIG_ObjectPreviouslyDeletedError;
+ }
+ }
+ }
if ((c = SWIG_MangleStr(obj)) == NULL) {
return SWIG_ERROR;
}
@@ -312,10 +320,6 @@ SWIG_Ruby_ConvertPtrAndOwn(VALUE obj, void **ptr, swig_type_info *ty, int flags,
if (!tc) {
return SWIG_ERROR;
} else {
- if (vptr == 0) {
- /* The object has already been deleted */
- return SWIG_ObjectPreviouslyDeletedError;
- }
if (ptr) {
if (tc->type == ty) {
*ptr = vptr;
--
2.5.5