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
This commit is contained in:
parent
dc1fad3f9b
commit
763827c2e1
5 changed files with 131 additions and 5 deletions
|
|
@ -5,6 +5,12 @@ See the RELEASENOTES file for a summary of changes in each release.
|
||||||
Version 3.0.9 (in progress)
|
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
|
2016-05-17: tamuratak
|
||||||
[Ruby] Patch #651 - Correct overloaded function error message when function is
|
[Ruby] Patch #651 - Correct overloaded function error message when function is
|
||||||
using %newobject.
|
using %newobject.
|
||||||
|
|
|
||||||
|
|
@ -30,7 +30,8 @@ CPP_TEST_CASES = \
|
||||||
# stl_new
|
# stl_new
|
||||||
|
|
||||||
C_TEST_CASES += \
|
C_TEST_CASES += \
|
||||||
li_cstring
|
li_cstring \
|
||||||
|
ruby_manual_proxy \
|
||||||
|
|
||||||
include $(srcdir)/../common.mk
|
include $(srcdir)/../common.mk
|
||||||
|
|
||||||
|
|
|
||||||
49
Examples/test-suite/ruby/ruby_manual_proxy_runme.rb
Normal file
49
Examples/test-suite/ruby/ruby_manual_proxy_runme.rb
Normal file
|
|
@ -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
|
||||||
66
Examples/test-suite/ruby_manual_proxy.i
Normal file
66
Examples/test-suite/ruby_manual_proxy.i
Normal file
|
|
@ -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);
|
||||||
|
}
|
||||||
|
%}
|
||||||
|
|
@ -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 */
|
/* Do type-checking if type info was provided */
|
||||||
if (ty) {
|
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) {
|
if ((c = SWIG_MangleStr(obj)) == NULL) {
|
||||||
return SWIG_ERROR;
|
return SWIG_ERROR;
|
||||||
}
|
}
|
||||||
|
|
@ -312,10 +320,6 @@ SWIG_Ruby_ConvertPtrAndOwn(VALUE obj, void **ptr, swig_type_info *ty, int flags,
|
||||||
if (!tc) {
|
if (!tc) {
|
||||||
return SWIG_ERROR;
|
return SWIG_ERROR;
|
||||||
} else {
|
} else {
|
||||||
if (vptr == 0) {
|
|
||||||
/* The object has already been deleted */
|
|
||||||
return SWIG_ObjectPreviouslyDeletedError;
|
|
||||||
}
|
|
||||||
if (ptr) {
|
if (ptr) {
|
||||||
if (tc->type == ty) {
|
if (tc->type == ty) {
|
||||||
*ptr = vptr;
|
*ptr = vptr;
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue