Skip to content

Commit

Permalink
Reject callback with :string return type
Browse files Browse the repository at this point in the history
Callbacks returning a :string type were not supported so far and did not work.
It was possible to define such a callback, but the value returned was NULL in any case.
This implementation rejects :string return type of callbacks at the definition.

The reason for the reject is the following:
In contrast to :string parameters to called functions there is no well defined ownership of the memory of the string returned by callbacks.
Instead an explicit FFI::MemoryPointer or similar should be used, which allows to track the validity of underlying memory instead of relying on some Ruby implementation details.

Fixes #751
  • Loading branch information
larskanis committed May 24, 2020
1 parent 6a14427 commit e2b1c7e
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 4 deletions.
3 changes: 1 addition & 2 deletions ext/ffi_c/FunctionInfo.c
Expand Up @@ -172,15 +172,14 @@ fntype_initialize(int argc, VALUE* argv, VALUE self)
VALUE typeName = rb_funcall2(rbReturnType, rb_intern("inspect"), 0, NULL);
rb_raise(rb_eTypeError, "Invalid return type (%s)", RSTRING_PTR(typeName));
}

if (rb_obj_is_kind_of(fnInfo->rbReturnType, rbffi_StructByValueClass)) {
fnInfo->hasStruct = true;
}

Data_Get_Struct(fnInfo->rbReturnType, Type, fnInfo->returnType);
fnInfo->ffiReturnType = fnInfo->returnType->ffiType;


#if defined(X86_WIN32)
rbConventionStr = (rbConvention != Qnil) ? rb_funcall2(rbConvention, rb_intern("to_s"), 0, NULL) : Qnil;
fnInfo->abi = (rbConventionStr != Qnil && strcmp(StringValueCStr(rbConventionStr), "stdcall") == 0)
Expand Down
6 changes: 5 additions & 1 deletion lib/ffi/library.rb
Expand Up @@ -394,7 +394,11 @@ def callback(*args)
options = Hash.new
options[:convention] = ffi_convention
options[:enums] = @ffi_enums if defined?(@ffi_enums)
cb = FFI::CallbackInfo.new(find_type(ret), native_params, options)
ret_type = find_type(ret)
if ret_type == FFI::Type::Builtin::STRING
raise TypeError, ":string is not allowed as return type of callbacks"
end
cb = FFI::CallbackInfo.new(ret_type, native_params, options)

# Add to the symbol -> type map (unless there was no name)
unless name.nil?
Expand Down
6 changes: 5 additions & 1 deletion lib/ffi/struct.rb
Expand Up @@ -228,7 +228,11 @@ def layout(*spec)

def callback(params, ret)
mod = enclosing_module
FFI::CallbackInfo.new(find_type(ret, mod), params.map { |e| find_type(e, mod) })
ret_type = find_type(ret, mod)
if ret_type == FFI::Type::Builtin::STRING
raise TypeError, ":string is not allowed as return type of callbacks"
end
FFI::CallbackInfo.new(ret_type, params.map { |e| find_type(e, mod) })
end

def packed(packed = 1)
Expand Down
9 changes: 9 additions & 0 deletions spec/ffi/callback_spec.rb
Expand Up @@ -309,6 +309,15 @@ class S8F32S32 < FFI::Struct
expect(s2[:f32]).to be_within(0.0000001).of 1.234567
end

it "returning :string is rejected as typedef" do
expect {
Module.new do
extend FFI::Library
ffi_lib TestLibrary::PATH
callback :cbVrA, [], :string
end
}.to raise_error(TypeError)
end

it "global variable" do
proc = Proc.new { 0x1e }
Expand Down
8 changes: 8 additions & 0 deletions spec/ffi/struct_callback_spec.rb
Expand Up @@ -66,4 +66,12 @@ class TestStruct < FFI::Struct
expect(fn.respond_to?(:call)).to be true
expect(fn.call(1, 2)).to eq(3)
end

it "callback returning :string is rejected in struct" do
expect {
Class.new(FFI::Struct) do
layout :function1, callback([:int, :int], :string)
end
}.to raise_error(TypeError)
end
end

0 comments on commit e2b1c7e

Please sign in to comment.