Fix read/writeString and add test (#243)

lune-org · Nov 12, 2024 · 7a51bc9 · 7a51bc9
1 parent e8cc2dc
commit 7a51bc9
Show file tree

Hide file tree

Showing 6 changed files with 83 additions and 34 deletions.
diff --git a/crates/lune-std-ffi/src/c/helper.rs b/crates/lune-std-ffi/src/c/helper.rs
@@ -14,6 +14,7 @@ pub mod method_provider {
     // Implement tostring
     pub fn provide_to_string<'lua, Target, M>(methods: &mut M)
     where
+        Target: 'static,
         M: LuaUserDataMethods<'lua, Target>,
     {
         methods.add_meta_function(LuaMetaMethod::ToString, |lua, this: LuaAnyUserData| {
@@ -24,6 +25,7 @@ pub mod method_provider {
     // Implement ptr method
     pub fn provide_ptr<'lua, Target, M>(methods: &mut M)
     where
+        Target: 'static,
         M: LuaUserDataMethods<'lua, Target>,
     {
         methods.add_function("ptr", |lua, this: LuaAnyUserData| {
@@ -34,6 +36,7 @@ pub mod method_provider {
     // Implement arr method
     pub fn provide_arr<'lua, Target, M>(methods: &mut M)
     where
+        Target: 'static,
         M: LuaUserDataMethods<'lua, Target>,
     {
         methods.add_function("arr", |lua, (this, length): (LuaAnyUserData, usize)| {
@@ -44,7 +47,7 @@ pub mod method_provider {
     // Implement readData method
     pub fn provide_read_data<'lua, Target, M>(methods: &mut M)
     where
-        Target: FfiSize + FfiConvert,
+        Target: FfiSize + FfiConvert + 'static,
         M: LuaUserDataMethods<'lua, Target>,
     {
         methods.add_method(
@@ -68,7 +71,7 @@ pub mod method_provider {
     // Implement writeData method
     pub fn provide_write_data<'lua, Target, M>(methods: &mut M)
     where
-        Target: FfiSize + FfiConvert,
+        Target: FfiSize + FfiConvert + 'static,
         M: LuaUserDataMethods<'lua, Target>,
     {
         methods.add_method(
@@ -93,7 +96,7 @@ pub mod method_provider {
     // Implement copyData method
     pub fn provide_copy_data<'lua, Target, M>(methods: &mut M)
     where
-        Target: FfiSize + FfiConvert,
+        Target: FfiSize + FfiConvert + 'static,
         M: LuaUserDataMethods<'lua, Target>,
     {
         methods.add_method(
@@ -134,7 +137,7 @@ pub mod method_provider {
     // Implement stringifyData method
     pub fn provide_stringify_data<'lua, Target, M>(methods: &mut M)
     where
-        Target: FfiSize + FfiConvert,
+        Target: FfiSize + FfiConvert + 'static,
         M: LuaUserDataMethods<'lua, Target>,
     {
         methods.add_method(
@@ -148,7 +151,7 @@ pub mod method_provider {
     // Implement box method
     pub fn provide_box<'lua, Target, M>(methods: &mut M)
     where
-        Target: FfiSize + FfiConvert,
+        Target: FfiSize + FfiConvert + 'static,
         M: LuaUserDataMethods<'lua, Target>,
     {
         methods.add_method("box", |lua, this, value: LuaValue| {

diff --git a/crates/lune-std-ffi/src/data/helper.rs b/crates/lune-std-ffi/src/data/helper.rs
@@ -8,19 +8,20 @@ pub mod method_provider {
     // Implement copyFrom method
     pub fn provide_copy_from<'lua, Target, M>(methods: &mut M)
     where
-        Target: FfiData,
+        Target: FfiData + 'static,
         M: LuaUserDataMethods<'lua, Target>,
     {
-        methods.add_method(
+        methods.add_function(
             "copyFrom",
             |_lua,
-             this,
-             (src, length, dst_offset, src_offset): (
+             (this_userdata, src, length, dst_offset, src_offset): (
+                LuaAnyUserData,
                 LuaAnyUserData,
                 usize,
                 Option<isize>,
                 Option<isize>,
             )| unsafe {
+                let this = this_userdata.borrow::<Target>()?;
                 let dst_offset = dst_offset.unwrap_or(0);
                 let src_offset = src_offset.unwrap_or(0);
                 let src = src.get_ffi_data()?;
@@ -34,15 +35,15 @@ pub mod method_provider {
 
                 this.copy_from(&src, length, dst_offset, src_offset);
 
-                Ok(())
+                Ok(this_userdata.clone())
             },
         );
     }
 
     // Implement readString method
     pub fn provide_read_string<'lua, Target, M>(methods: &mut M)
     where
-        Target: FfiData,
+        Target: FfiData + 'static,
         M: LuaUserDataMethods<'lua, Target>,
     {
         methods.add_method(
@@ -62,24 +63,27 @@ pub mod method_provider {
     // Implement writeString method
     pub fn provide_write_string<'lua, Target, M>(methods: &mut M)
     where
-        Target: FfiData,
+        Target: FfiData + 'static,
         M: LuaUserDataMethods<'lua, Target>,
     {
-        methods.add_method(
+        methods.add_function(
             "writeString",
             |_lua,
-             this,
-             (string, length, dst_offset, src_offset): (
+             (this_userdata, string, length, dst_offset, src_offset): (
+                LuaAnyUserData,
                 LuaString,
-                usize,
+                Option<usize>,
                 Option<isize>,
                 Option<usize>,
             )| unsafe {
+                let string_len = string.as_bytes().len();
                 let dst_offset = dst_offset.unwrap_or(0);
                 let src_offset = src_offset.unwrap_or(0);
+                let length = length.unwrap_or_else(|| string_len - src_offset);
+                let this = this_userdata.borrow::<Target>()?;
 
                 // Source string boundary check
-                if string.as_bytes().len() < src_offset + length {
+                if string_len < src_offset + length {
                     return Err(LuaError::external("Source out of bounds"));
                 }
 
@@ -89,7 +93,7 @@ pub mod method_provider {
                 }
 
                 this.write_string(string, length, dst_offset, src_offset);
-                Ok(())
+                Ok(this_userdata.clone())
             },
         );
     }

diff --git a/crates/lune-std-ffi/src/ffi/mod.rs b/crates/lune-std-ffi/src/ffi/mod.rs
@@ -97,7 +97,10 @@ pub trait FfiData {
         self.get_inner_pointer()
             .cast::<u8>()
             .byte_offset(dst_offset)
-            .copy_from(src.to_pointer().cast::<u8>().byte_add(src_offset), length);
+            .copy_from(
+                src.as_bytes().as_ptr().cast::<u8>().byte_add(src_offset),
+                length,
+            );
     }
 }
 

diff --git a/crates/lune/src/tests.rs b/crates/lune/src/tests.rs
@@ -123,6 +123,7 @@ create_tests! {
     ffi_free: "ffi/free",
     ffi_is_integer: "ffi/isInteger",
     ffi_read_boundary: "ffi/readBoundary",
+    ffi_read_write_string: "ffi/stringReadWrite",
     ffi_write_boundary: "ffi/writeBoundary",
 }
 

diff --git a/tests/ffi/stringReadWrite.luau b/tests/ffi/stringReadWrite.luau
@@ -0,0 +1,34 @@
+local ffi = require("@lune/ffi")
+local ok
+
+local str = "hello world"
+local strbox = ffi.box(#str):writeString(str)
+assert(strbox:readString(#str) == str, "String read write assersion failed")
+
+-- Case1: Fail
+ok = pcall(function()
+	local box = ffi.box(2)
+	box:readString(10)
+end)
+assert(not ok, "assersion failed, Case1 should fail")
+
+-- Case2: Fail
+ok = pcall(function()
+	local box = ffi.box(2)
+	box:writeString("hello world")
+end)
+assert(not ok, "assersion failed, Case2 should fail")
+
+-- Case3: Fail
+ok = pcall(function()
+	local box = ffi.box(2):ref()
+	box:readString(10)
+end)
+assert(not ok, "assersion failed, Case3 should fail")
+
+-- Case4: Fail
+ok = pcall(function()
+	local box = ffi.box(2):ref()
+	box:writeString("hello world")
+end)
+assert(not ok, "assersion failed, Case4 should fail")
diff --git a/types/ffi.luau b/types/ffi.luau
@@ -128,20 +128,21 @@ export type RefData = {
 		@param length The amount of data to copy, in bytes
 		@param dstOffset The offset in the destination where the content will be pasted
 		@param srcOffset The offset in the source data from where the content will be copied
+		@return `RefData` itself for convenience
 	]=]
 	copyFrom: (
 		self: RefData,
 		src: BoxData | RefData,
 		length: number,
 		dstOffset: number,
 		srcOffset: number
-	) -> (),
+	) -> RefData,
 	--[=[
 		@within RefData
 		@tag Method
 		@method readString
 
-		Read string from data with specific length.
+		Read string from data with specific length without null termination.
 
 		@param length The amount of data to read, in bytes
 		@param offset Offset to read string from
@@ -153,20 +154,21 @@ export type RefData = {
 		@tag Method
 		@method writeString
 
-		Write string into data.
+		Write string into data without null termination.
 
 		@param src The source string
 		@param length The amount of data to write, in bytes
 		@param dstOffset The offset in the destination where the content will be pasted
 		@param srcOffset The offset in the source string from where the content will be copied
+		@return `RefData` itself for convenience
 	]=]
 	writeString: (
 		self: RefData,
 		src: string,
-		length: number,
-		dstOffset: number,
-		srcOffset: number
-	) -> (),
+		length: number?,
+		dstOffset: number?,
+		srcOffset: number?
+	) -> RefData,
 }
 
 --[=[
@@ -191,7 +193,7 @@ export type BoxData = {
 
 		Fill the box with zero.
 
-		@return `Box` itself for convenience
+		@return `BoxData` itself for convenience
 	]=]
 	zero: (self: BoxData) -> BoxData,
 	--[=[
@@ -231,20 +233,21 @@ export type BoxData = {
 		@param length The amount of data to copy, in bytes
 		@param dstOffset The offset in the destination where the content will be pasted
 		@param srcOffset The offset in the source data from where the content will be copied
+		@return `BoxData` itself for convenience
 	]=]
 	copyFrom: (
 		self: BoxData,
 		src: BoxData | RefData,
 		length: number,
 		dstOffset: number,
 		srcOffset: number
-	) -> (),
+	) -> BoxData,
 	--[=[
 		@within BoxData
 		@tag Method
 		@method readString
 
-		Read string from data with specific length.
+		Read string from data with specific length without null termination.
 
 		@param length The amount of data to read, in bytes
 		@param offset Offset to read string from
@@ -256,20 +259,21 @@ export type BoxData = {
 		@tag Method
 		@method writeString
 
-		Write string into data.
+		Write string into data without null termination.
 
 		@param src The source string
 		@param length The amount of data to write, in bytes
 		@param dstOffset The offset in the destination where the content will be pasted
 		@param srcOffset The offset in the source string from where the content will be copied
+		@return `BoxData` itself for convenience
 	]=]
 	writeString: (
 		self: BoxData,
 		src: string,
-		length: number,
-		dstOffset: number,
-		srcOffset: number
-	) -> (),
+		length: number?,
+		dstOffset: number?,
+		srcOffset: number?
+	) -> BoxData,
 }
 
 --[=[