From 32df97ec2b33a4d1e8d2cf8607a6490d88bcd9c6 Mon Sep 17 00:00:00 2001
From: Lars Kanis <lars@greiz-reinsdorf.de>
Date: Thu, 16 Apr 2020 13:59:08 +0200
Subject: [PATCH] Implement callback returning type :string

Callbacks returning a :string type were not supported so far.
It was possible to define such a callback, but the value returned was NULL in any case.
This implementation follows the semantics for passing :string parameters to functions as implemented in Call.c.
That means in particular usage of StringValueCStr(), so that no zero bytes are allowed in the string.

Special care has to be taken regarding lifetime of the returned text pointer.
The C pointer to the string is only valid until the next GC triggering function is called, unless the String object is hold in Ruby outside of the callback block.
Even when the object is stored outside of the Ruby block, the text pointer can move when GC.compact is called.
So :string returning callbacks can safely be used only, when the referenced text is processed immediately by the calling C function.

Resolves #751
---
 ext/ffi_c/Function.c      |  9 +++++++++
 spec/ffi/callback_spec.rb | 11 +++++++++++
 2 files changed, 20 insertions(+)

diff --git a/ext/ffi_c/Function.c b/ext/ffi_c/Function.c
index 177180003..c2d34d8a9 100644
--- a/ext/ffi_c/Function.c
+++ b/ext/ffi_c/Function.c
@@ -793,6 +793,15 @@ invoke_callback(VALUE data)
         case NATIVE_FLOAT64:
             *((double *) retval) = NUM2DBL(rbReturnValue);
             break;
+
+        case NATIVE_STRING:
+            if (NIL_P(rbReturnValue)) {
+                *((void **) retval) = NULL;
+            } else {
+                *((void **) retval) = StringValueCStr(rbReturnValue);
+            }
+            break;
+
         case NATIVE_POINTER:
             if (TYPE(rbReturnValue) == T_DATA && rb_obj_is_kind_of(rbReturnValue, rbffi_PointerClass)) {
                 *((void **) retval) = ((AbstractMemory *) DATA_PTR(rbReturnValue))->address;
diff --git a/spec/ffi/callback_spec.rb b/spec/ffi/callback_spec.rb
index be79ad2ff..26948daa2 100644
--- a/spec/ffi/callback_spec.rb
+++ b/spec/ffi/callback_spec.rb
@@ -51,6 +51,7 @@ class S8F32S32 < FFI::Struct
       callback :cbVrUL, [ ], :ulong
       callback :cbVrS64, [ ], :long_long
       callback :cbVrU64, [ ], :ulong_long
+      callback :cbVrA, [], :string
       callback :cbVrP, [], :pointer
       callback :cbVrZ, [], :bool
       callback :cbCrV, [ :char ], :void
@@ -75,6 +76,7 @@ class S8F32S32 < FFI::Struct
       attach_function :testCallbackVrUL, :testClosureVrL, [ :cbVrUL ], :ulong
       attach_function :testCallbackVrS64, :testClosureVrLL, [ :cbVrS64 ], :long_long
       attach_function :testCallbackVrU64, :testClosureVrLL, [ :cbVrU64 ], :ulong_long
+      attach_function :testCallbackVrA, :testClosureVrP, [ :cbVrA ], :string
       attach_function :testCallbackVrP, :testClosureVrP, [ :cbVrP ], :pointer
       attach_function :testCallbackReturningFunction, :testClosureVrP, [ :cbVrP ], :cbVrP
       attach_function :testCallbackVrY, :testClosureVrP, [ :cbVrY ], S8F32S32.ptr
@@ -261,6 +263,15 @@ class S8F32S32 < FFI::Struct
       expect(LibTest.testCallbackVrZ { true }).to be true
     end
 
+    it "returning :string (nil)" do
+      expect(LibTest.testCallbackVrA { nil }).to be_nil
+    end
+
+    it "returning :string" do
+      str = "String ä"
+      expect(LibTest.testCallbackVrA { str }).to eq(str.b)
+    end
+
     it "returning :pointer (nil)" do
       expect(LibTest.testCallbackVrP { nil }).to be_null
     end