From f757c9efacfaa6ba3771b03b6e27d029e964f5af Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Thu, 24 Sep 2015 08:02:31 -0700 Subject: [PATCH] Better embedding API support (and tests!) for strings with null bytes. - Test that a foreign method can return strings. - Test that a foreign method can return a string with null bytes. - Test wrenCall(). - Allow passing NULL for "v" to wrenCall(). - Allow "a" for passing an explicit length byte array to wrenCall(). --- src/cli/vm.c | 6 +-- src/cli/vm.h | 4 +- src/include/wren.h | 18 ++++++-- src/vm/wren_vm.c | 18 ++++++-- test/api/call.c | 50 +++++++++++++++++++++++ test/api/call.h | 3 ++ test/api/call.wren | 45 ++++++++++++++++++++ test/api/main.c | 5 ++- test/api/returns.c | 12 ++++++ test/api/returns.wren | 6 +++ util/xcode/wren.xcodeproj/project.pbxproj | 6 +++ 11 files changed, 159 insertions(+), 14 deletions(-) create mode 100644 test/api/call.c create mode 100644 test/api/call.h create mode 100644 test/api/call.wren diff --git a/src/cli/vm.c b/src/cli/vm.c index 64dcee5f..054a8ce5 100644 --- a/src/cli/vm.c +++ b/src/cli/vm.c @@ -172,7 +172,7 @@ static void initVM() static void freeVM() { schedulerReleaseMethods(); - + uv_loop_close(loop); free(loop); @@ -259,8 +259,8 @@ uv_loop_t* getLoop() return loop; } -void setForeignCallbacks( - WrenBindForeignMethodFn bindMethod, WrenBindForeignClassFn bindClass) +void setTestCallbacks(WrenBindForeignMethodFn bindMethod, + WrenBindForeignClassFn bindClass) { bindMethodFn = bindMethod; bindClassFn = bindClass; diff --git a/src/cli/vm.h b/src/cli/vm.h index 1ccf86b9..578c8dda 100644 --- a/src/cli/vm.h +++ b/src/cli/vm.h @@ -22,7 +22,7 @@ uv_loop_t* getLoop(); // // Used by the API test executable to let it wire up its own foreign functions. // This must be called before calling [createVM()]. -void setForeignCallbacks(WrenBindForeignMethodFn bindMethod, - WrenBindForeignClassFn bindClass); +void setTestCallbacks(WrenBindForeignMethodFn bindMethod, + WrenBindForeignClassFn bindClass); #endif diff --git a/src/include/wren.h b/src/include/wren.h index 40fd47c0..609f1e6d 100644 --- a/src/include/wren.h +++ b/src/include/wren.h @@ -213,8 +213,15 @@ WrenValue* wrenGetMethod(WrenVM* vm, const char* module, const char* variable, // will allocate its own string and copy the characters from this, so // you don't have to worry about the lifetime of the string you pass to // Wren. +// - "a" - An array of bytes converted to a Wren String. This requires two +// consecutive arguments in the argument list: `const char*` pointing +// to the array of bytes, followed by an `int` defining the length of +// the array. This is used when the passed string may contain null +// bytes, or just to avoid the implicit `strlen()` call of "s" if you +// happen to already know the length. // - "v" - A previously acquired WrenValue*. Passing this in does not implicitly -// release the value. +// release the value. If the passed argument is NULL, this becomes a +// Wren NULL. // // [method] must have been created by a call to [wrenGetMethod]. If [result] is // not `NULL`, the return value of the method will be stored in a new @@ -302,9 +309,12 @@ void wrenReturnDouble(WrenVM* vm, double value); // Provides a string return value for a foreign call. // // The [text] will be copied to a new string within Wren's heap, so you can -// free memory used by it after this is called. If [length] is non-zero, Wren -// will copy that many bytes from [text]. If it is -1, then the length of -// [text] will be calculated using `strlen()`. +// free memory used by it after this is called. +// +// If [length] is non-zero, Wren copies that many bytes from [text], including +// any null bytes. If it is -1, then the length of [text] is calculated using +// `strlen()`. If the string may contain any null bytes in the middle, then you +// must pass an explicit length. void wrenReturnString(WrenVM* vm, const char* text, int length); // Provides the return value for a foreign call. diff --git a/src/vm/wren_vm.c b/src/vm/wren_vm.c index 7e902ebf..0b71263b 100644 --- a/src/vm/wren_vm.c +++ b/src/vm/wren_vm.c @@ -1334,17 +1334,29 @@ WrenInterpretResult wrenCall(WrenVM* vm, WrenValue* method, WrenValue** result, Value value = NULL_VAL; switch (*argType) { + case 'a': + { + const char* bytes = va_arg(argList, const char*); + int length = va_arg(argList, int); + value = wrenNewString(vm, bytes, (size_t)length); + break; + } + case 'b': value = BOOL_VAL(va_arg(argList, int)); break; case 'd': value = NUM_VAL(va_arg(argList, double)); break; case 'i': value = NUM_VAL((double)va_arg(argList, int)); break; case 'n': value = NULL_VAL; va_arg(argList, void*); break; case 's': - { value = wrenStringFormat(vm, "$", va_arg(argList, const char*)); break; - } - case 'v': value = va_arg(argList, WrenValue*)->value; break; + case 'v': + { + // Allow a NULL value pointer for Wren null. + WrenValue* wrenValue = va_arg(argList, WrenValue*); + if (wrenValue != NULL) value = wrenValue->value; + break; + } default: ASSERT(false, "Unknown argument type."); diff --git a/test/api/call.c b/test/api/call.c new file mode 100644 index 00000000..21c4c847 --- /dev/null +++ b/test/api/call.c @@ -0,0 +1,50 @@ +#include + +#include "call.h" +#include "vm.h" + +static void runTests(WrenVM* vm) +{ + WrenValue* noParams = wrenGetMethod(vm, "main", "Api", "noParams"); + WrenValue* zero = wrenGetMethod(vm, "main", "Api", "zero()"); + WrenValue* one = wrenGetMethod(vm, "main", "Api", "one(_)"); + WrenValue* two = wrenGetMethod(vm, "main", "Api", "two(_,_)"); + + // Different arity. + wrenCall(vm, noParams, NULL, ""); + wrenCall(vm, zero, NULL, ""); + wrenCall(vm, one, NULL, "i", 1); + wrenCall(vm, two, NULL, "ii", 1, 2); + + WrenValue* getValue = wrenGetMethod(vm, "main", "Api", "getValue(_)"); + + // Returning a value. + WrenValue* value = NULL; + wrenCall(vm, getValue, &value, "v", NULL); + + // Different argument types. + wrenCall(vm, two, NULL, "bb", true, false); + wrenCall(vm, two, NULL, "dd", 1.2, 3.4); + wrenCall(vm, two, NULL, "ii", 3, 4); + wrenCall(vm, two, NULL, "ss", "string", "another"); + wrenCall(vm, two, NULL, "vv", NULL, value); + + // Truncate a string, or allow null bytes. + wrenCall(vm, two, NULL, "aa", "string", 3, "b\0y\0t\0e", 7); + + wrenReleaseValue(vm, noParams); + wrenReleaseValue(vm, zero); + wrenReleaseValue(vm, one); + wrenReleaseValue(vm, two); + wrenReleaseValue(vm, getValue); + wrenReleaseValue(vm, value); +} + + +WrenForeignMethodFn callBindMethod(const char* signature) +{ + if (strcmp(signature, "static Api.runTests()") == 0) return runTests; + + return NULL; +} + diff --git a/test/api/call.h b/test/api/call.h new file mode 100644 index 00000000..f4961b70 --- /dev/null +++ b/test/api/call.h @@ -0,0 +1,3 @@ +#include "wren.h" + +WrenForeignMethodFn callBindMethod(const char* signature); diff --git a/test/api/call.wren b/test/api/call.wren new file mode 100644 index 00000000..fc7126d8 --- /dev/null +++ b/test/api/call.wren @@ -0,0 +1,45 @@ +class Api { + static noParams { + System.print("noParams") + } + + static zero() { + System.print("zero") + } + + static one(one) { + System.print("one " + one.toString) + } + + static two(one, two) { + // Don't print null bytes. + if (two is String && two.bytes.contains(0)) { + two = two.bytes.toList + } + + System.print("two " + one.toString + " " + two.toString) + } + + static getValue(value) { + // Return a new value if we aren't given one. + if (value == null) return ["a", "b"] + + // Otherwise print it. + System.print(value) + } + + foreign static runTests() +} + +Api.runTests() +// expect: noParams +// expect: zero +// expect: one 1 +// expect: two 1 2 + +// expect: two true false +// expect: two 1.2 3.4 +// expect: two 3 4 +// expect: two string another +// expect: two null [a, b] +// expect: two str [98, 0, 121, 0, 116, 0, 101] diff --git a/test/api/main.c b/test/api/main.c index a7b7f4c7..e2dcc9e4 100644 --- a/test/api/main.c +++ b/test/api/main.c @@ -4,6 +4,7 @@ #include "vm.h" #include "wren.h" +#include "call.h" #include "foreign_class.h" #include "returns.h" #include "value.h" @@ -35,6 +36,7 @@ static WrenForeignMethodFn bindForeignMethod( strcat(fullName, "."); strcat(fullName, signature); + REGISTER_METHOD(call, call); REGISTER_METHOD(foreign_class, foreignClass); REGISTER_METHOD(returns, returns); REGISTER_METHOD(value, value); @@ -56,7 +58,6 @@ static WrenForeignClassMethods bindForeignClass( return methods; } - int main(int argc, const char* argv[]) { if (argc != 2) @@ -73,7 +74,7 @@ int main(int argc, const char* argv[]) strcat(testPath, testName); strcat(testPath, ".wren"); - setForeignCallbacks(bindForeignMethod, bindForeignClass); + setTestCallbacks(bindForeignMethod, bindForeignClass); runFile(testPath); return 0; } diff --git a/test/api/returns.c b/test/api/returns.c index 0e7d0700..e1ba9155 100644 --- a/test/api/returns.c +++ b/test/api/returns.c @@ -27,6 +27,16 @@ static void returnFalse(WrenVM* vm) wrenReturnBool(vm, false); } +static void returnString(WrenVM* vm) +{ + wrenReturnString(vm, "a string", -1); +} + +static void returnBytes(WrenVM* vm) +{ + wrenReturnString(vm, "a\0b\0c", 5); +} + WrenForeignMethodFn returnsBindMethod(const char* signature) { if (strcmp(signature, "static Api.implicitNull") == 0) return implicitNull; @@ -34,6 +44,8 @@ WrenForeignMethodFn returnsBindMethod(const char* signature) if (strcmp(signature, "static Api.returnFloat") == 0) return returnFloat; if (strcmp(signature, "static Api.returnTrue") == 0) return returnTrue; if (strcmp(signature, "static Api.returnFalse") == 0) return returnFalse; + if (strcmp(signature, "static Api.returnString") == 0) return returnString; + if (strcmp(signature, "static Api.returnBytes") == 0) return returnBytes; return NULL; } diff --git a/test/api/returns.wren b/test/api/returns.wren index dade69ec..b50edfcc 100644 --- a/test/api/returns.wren +++ b/test/api/returns.wren @@ -6,6 +6,9 @@ class Api { foreign static returnTrue foreign static returnFalse + + foreign static returnString + foreign static returnBytes } System.print(Api.implicitNull == null) // expect: true @@ -15,3 +18,6 @@ System.print(Api.returnFloat) // expect: 123.456 System.print(Api.returnTrue) // expect: true System.print(Api.returnFalse) // expect: false + +System.print(Api.returnString) // expect: a string +System.print(Api.returnBytes.bytes.toList) // expect: [97, 0, 98, 0, 99] diff --git a/util/xcode/wren.xcodeproj/project.pbxproj b/util/xcode/wren.xcodeproj/project.pbxproj index 606235da..d7c6d816 100644 --- a/util/xcode/wren.xcodeproj/project.pbxproj +++ b/util/xcode/wren.xcodeproj/project.pbxproj @@ -20,6 +20,7 @@ 29205C9D1AB4E6430073018D /* wren_utils.c in Sources */ = {isa = PBXBuildFile; fileRef = 29205C961AB4E6430073018D /* wren_utils.c */; }; 29205C9E1AB4E6430073018D /* wren_value.c in Sources */ = {isa = PBXBuildFile; fileRef = 29205C971AB4E6430073018D /* wren_value.c */; }; 29205C9F1AB4E6430073018D /* wren_vm.c in Sources */ = {isa = PBXBuildFile; fileRef = 29205C981AB4E6430073018D /* wren_vm.c */; }; + 293D46961BB43F9900200083 /* call.c in Sources */ = {isa = PBXBuildFile; fileRef = 293D46941BB43F9900200083 /* call.c */; }; 29512C811B91F8EB008C10E6 /* libuv.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 29512C801B91F8EB008C10E6 /* libuv.a */; }; 29512C821B91F901008C10E6 /* libuv.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 29512C801B91F8EB008C10E6 /* libuv.a */; }; 29729F311BA70A620099CA20 /* io.c in Sources */ = {isa = PBXBuildFile; fileRef = 29729F2E1BA70A620099CA20 /* io.c */; }; @@ -75,6 +76,8 @@ 29205CA61AB4E65E0073018D /* wren_utils.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = wren_utils.h; path = ../../src/vm/wren_utils.h; sourceTree = ""; }; 29205CA71AB4E65E0073018D /* wren_value.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = wren_value.h; path = ../../src/vm/wren_value.h; sourceTree = ""; }; 29205CA81AB4E65E0073018D /* wren_vm.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = wren_vm.h; path = ../../src/vm/wren_vm.h; sourceTree = ""; }; + 293D46941BB43F9900200083 /* call.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; name = call.c; path = ../../test/api/call.c; sourceTree = ""; }; + 293D46951BB43F9900200083 /* call.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = call.h; path = ../../test/api/call.h; sourceTree = ""; }; 29512C7F1B91F86E008C10E6 /* api_test */ = {isa = PBXFileReference; explicitFileType = "compiled.mach-o.executable"; includeInIndex = 0; path = api_test; sourceTree = BUILT_PRODUCTS_DIR; }; 29512C801B91F8EB008C10E6 /* libuv.a */ = {isa = PBXFileReference; lastKnownFileType = archive.ar; name = libuv.a; path = ../../build/libuv.a; sourceTree = ""; }; 296371B31AC713D000079FDA /* wren_opcodes.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = wren_opcodes.h; path = ../../src/vm/wren_opcodes.h; sourceTree = ""; }; @@ -201,6 +204,8 @@ isa = PBXGroup; children = ( 29D009A61B7E3993000CE58C /* main.c */, + 293D46941BB43F9900200083 /* call.c */, + 293D46951BB43F9900200083 /* call.h */, 29D009A81B7E39A8000CE58C /* foreign_class.c */, 29D009A91B7E39A8000CE58C /* foreign_class.h */, 29D009AA1B7E39A8000CE58C /* returns.c */, @@ -310,6 +315,7 @@ 29729F341BA70A620099CA20 /* io.wren.inc in Sources */, 291647C81BA5EC5E006142EE /* modules.c in Sources */, 29729F321BA70A620099CA20 /* io.c in Sources */, + 293D46961BB43F9900200083 /* call.c in Sources */, 291647D21BA5ED26006142EE /* timer.wren.inc in Sources */, 291647D01BA5ED26006142EE /* scheduler.wren.inc in Sources */, );