From 7a51bc9aa4a5f2d3359b2f24692958781e42edae Mon Sep 17 00:00:00 2001 From: qwreey Date: Tue, 12 Nov 2024 06:34:22 +0000 Subject: [PATCH] Fix read/writeString and add test (#243) --- crates/lune-std-ffi/src/c/helper.rs | 13 ++++++---- crates/lune-std-ffi/src/data/helper.rs | 30 +++++++++++++---------- crates/lune-std-ffi/src/ffi/mod.rs | 5 +++- crates/lune/src/tests.rs | 1 + tests/ffi/stringReadWrite.luau | 34 ++++++++++++++++++++++++++ types/ffi.luau | 34 ++++++++++++++------------ 6 files changed, 83 insertions(+), 34 deletions(-) create mode 100644 tests/ffi/stringReadWrite.luau diff --git a/crates/lune-std-ffi/src/c/helper.rs b/crates/lune-std-ffi/src/c/helper.rs index 1e246531..b56ed7f8 100644 --- 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 index 0d48e1c9..0b7de25b 100644 --- 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, Option, )| unsafe { + let this = this_userdata.borrow::()?; let dst_offset = dst_offset.unwrap_or(0); let src_offset = src_offset.unwrap_or(0); let src = src.get_ffi_data()?; @@ -34,7 +35,7 @@ pub mod method_provider { this.copy_from(&src, length, dst_offset, src_offset); - Ok(()) + Ok(this_userdata.clone()) }, ); } @@ -42,7 +43,7 @@ pub mod method_provider { // 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, Option, Option, )| 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::()?; // 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 index dc928305..5d6073a7 100644 --- 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::() .byte_offset(dst_offset) - .copy_from(src.to_pointer().cast::().byte_add(src_offset), length); + .copy_from( + src.as_bytes().as_ptr().cast::().byte_add(src_offset), + length, + ); } } diff --git a/crates/lune/src/tests.rs b/crates/lune/src/tests.rs index 4d06e1b1..0a62219e 100644 --- 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 new file mode 100644 index 00000000..43510475 --- /dev/null +++ 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 index d0dbd75b..a4238c7d 100644 --- a/types/ffi.luau +++ b/types/ffi.luau @@ -128,6 +128,7 @@ 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, @@ -135,13 +136,13 @@ export type 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,6 +233,7 @@ 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, @@ -238,13 +241,13 @@ export type BoxData = { 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, } --[=[