Fix(runner): thread array does not resize even if allocated (#144)

`runner_impl` uses `set` which does not support resize; therefore,
resize logic was added iff the thread array is full.

fixes: #142

Author: JBenda

CMakeLists.txt

@@ -48,7 +48,7 @@ set(CMAKE_INSTALL_LIBRARY_DIR lib)
 set(CMAKE_INSTALL_INCLUDE_DIR include)
 if(MSVC)
 	# disable: aligment problems non explicit switch case
-	add_compile_options(/W4 /wd4820 /wd4061)
+	add_compile_options(/W4 /wd4820 /wd4061 -D_CRT_SECURE_NO_WARNINGS)
 else()
 	add_compile_options(-Wall -Wextra -Wpedantic)
 endif()

inkcpp/array.h

@@ -464,7 +464,7 @@ class fixed_restorable_array : public basic_restorable_array<T>
 };
 
 template<typename T>
-class allocated_restorable_array final : public basic_restorable_array<T>
+class allocated_restorable_array : public basic_restorable_array<T>
 {
 	using base = basic_restorable_array<T>;
 

inkcpp/runner_impl.h

@@ -273,7 +273,14 @@ class runner_impl
 			_threadDone.forget();
 		}
 
-		void set(size_t index, const ip_t& value) { _threadDone.set(index, value); }
+		void set(size_t index, const ip_t& value)
+		{
+			if (index >= _threadDone.capacity()) {
+				inkAssert(index == _threadDone.capacity(), "Threads should only be created incremental");
+				_threadDone.resize(static_cast<size_t>(_threadDone.capacity() * 1.5));
+			}
+			_threadDone.set(index, value);
+		}
 
 		const ip_t& get(size_t index) const { return _threadDone.get(index); }
 

inkcpp_c/include/inkcpp.h

@@ -3,7 +3,7 @@
 
 #include <stdbool.h>
 #include <stdint.h>
-#include <stdlib.h>
+#include <stddef.h>
 
 #ifdef __cplusplus
 extern "C" {

inkcpp_c/inkcpp.cpp

@@ -74,13 +74,16 @@ extern "C" {
 		FILE* file = fopen(filename, "rb");
 		fseek(file, 0, SEEK_END);
 		long file_length = ftell(file);
+		if (file_length < 0) {
+			return NULL;
+		}
 		fseek(file, 0, SEEK_SET);
 		unsigned char* data = static_cast<unsigned char*>(malloc(file_length));
 		inkAssert(data, "Malloc of size %u failed", file_length);
-		unsigned length = fread(data, sizeof(unsigned char), file_length, file);
+		size_t length = fread(data, sizeof(unsigned char), file_length, file);
 		inkAssert(
-		    file_length == length, "Expected to read file of size %u, but only read %u", file_length,
-		    length
+		    static_cast<size_t>(file_length) == length,
+		    "Expected to read file of size %u, but only read %u", file_length, length
 		);
 		fclose(file);
 		return reinterpret_cast<HInkStory*>(story::from_binary(data, file_length));
@@ -91,13 +94,16 @@ extern "C" {
 		FILE* file = fopen(filename, "rb");
 		fseek(file, 0, SEEK_END);
 		long file_length = ftell(file);
+		if (file_length < 0) {
+			return NULL;
+		}
 		fseek(file, 0, SEEK_SET);
 		unsigned char* data = static_cast<unsigned char*>(malloc(file_length));
 		inkAssert(data, "Malloc of size %u failed", file_length);
-		unsigned length = fread(data, sizeof(unsigned char), file_length, file);
+		size_t length = fread(data, sizeof(unsigned char), file_length, file);
 		inkAssert(
-		    file_length == length, "Expected to read file of size %u, but only read %u", file_length,
-		    length
+		    static_cast<size_t>(file_length) == length,
+		    "Expected to read file of size %u, but only read %u", file_length, length
 		);
 		fclose(file);
 		return reinterpret_cast<HInkSnapshot*>(snapshot::from_binary(data, file_length));
@@ -107,7 +113,7 @@ extern "C" {
 	{
 		FILE*           file = fopen(filename, "wb");
 		const snapshot& snap = *reinterpret_cast<const snapshot*>(self);
-		unsigned length = fwrite(snap.get_data(), sizeof(unsigned char), snap.get_data_len(), file);
+		size_t length = fwrite(snap.get_data(), sizeof(unsigned char), snap.get_data_len(), file);
 		inkAssert(
 		    length == snap.get_data_len(),
 		    "Snapshot write failed, snapshot of size %u, but only %u bytes where written.",
@@ -119,13 +125,17 @@ extern "C" {
 
 	HInkStory* ink_story_from_binary(const unsigned char* data, size_t length, bool freeOnDestroy)
 	{
-		return reinterpret_cast<HInkStory*>(story::from_binary(data, length, freeOnDestroy));
+		return reinterpret_cast<HInkStory*>(
+		    story::from_binary(data, static_cast<ink::size_t>(length), freeOnDestroy)
+		);
 	}
 
 	HInkSnapshot*
 	    ink_snapshot_from_binary(const unsigned char* data, size_t length, bool freeOnDestroy)
 	{
-		return reinterpret_cast<HInkSnapshot*>(snapshot::from_binary(data, length, freeOnDestroy));
+		return reinterpret_cast<HInkSnapshot*>(
+		    snapshot::from_binary(data, static_cast<ink::size_t>(length), freeOnDestroy)
+		);
 	}
 
 	void ink_snapshot_get_binary(
@@ -291,7 +301,7 @@ extern "C" {
 		    function_name,
 		    [callback](size_t len, const value* vals) {
 			    InkValue* c_vals = reinterpret_cast<InkValue*>(const_cast<value*>(vals));
-			    int       c_len  = len;
+			    int       c_len  = static_cast<int>(len);
 			    for (int i = 0; i < c_len; ++i) {
 				    c_vals[i] = inkvar_to_c(const_cast<value&>(vals[i]));
 			    }
@@ -310,7 +320,7 @@ extern "C" {
 		    function_name,
 		    [callback](size_t len, const value* vals) -> value {
 			    InkValue* c_vals = reinterpret_cast<InkValue*>(const_cast<value*>(vals));
-			    int       c_len  = len;
+			    int       c_len  = static_cast<int>(len);
 			    for (int i = 0; i < c_len; ++i) {
 				    c_vals[i] = inkvar_to_c(const_cast<value&>(vals[i]));
 			    }

inkcpp_c/tests/ExternalFunction.c

@@ -17,17 +17,18 @@ InkValue my_sqrt(int argc, const InkValue argv[])
 	InkValue v = argv[0];
 	switch (v.type) {
 		case ValueTypeFloat: v.float_v = sqrtf(v.float_v); break;
-		case ValueTypeInt32: v.int32_v = sqrt(v.int32_v); break;
-		case ValueTypeUint32: v.uint32_v = sqrtf(v.uint32_v); break;
+		case ValueTypeInt32: v.int32_v = ( int32_t ) sqrt(v.int32_v); break;
+		case ValueTypeUint32: v.uint32_v = ( uint32_t ) sqrt(v.uint32_v); break;
 		default: assert(0);
 	}
 	return v;
 }
 
 int cnt_greeting = 0;
 
-InkValue greeting(int argc, const InkValue argv[])
+InkValue greeting(int argc, const InkValue* argv)
 {
+	( void ) argv;
 	cnt_greeting += 1;
 	assert(argc == 0);
 	InkValue v;
@@ -38,6 +39,8 @@ InkValue greeting(int argc, const InkValue argv[])
 
 int main(int argc, const char* argv[])
 {
+	( void ) argc;
+	( void ) argv;
 	HInkStory*  story  = ink_story_from_file(INK_TEST_RESOURCE_DIR "FallBack.bin");
 	HInkRunner* runner = ink_story_new_runner(story, NULL);
 	ink_runner_bind(runner, "greeting", greeting, 0);

inkcpp_test/Array.cpp

@@ -12,12 +12,11 @@ SCENARIO("a restorable array can hold values", "[array]")
 	GIVEN("an empty array")
 	{
 		const ink::size_t length = 10;
-		test_array array = test_array(length, 0, ~0);
+		test_array        array  = test_array(length, 0U, ~0U);
 
 		THEN("the default values should be zero")
 		{
-			for (ink::size_t i = 0; i < length; i++)
-			{
+			for (ink::size_t i = 0; i < length; i++) {
 				REQUIRE(array[i] == 0);
 			}
 		}
@@ -26,15 +25,11 @@ SCENARIO("a restorable array can hold values", "[array]")
 		{
 			array.set(3, 15);
 
-			THEN("the value should be set")
-			{
-				REQUIRE(array[3] == 15);
-			}
+			THEN("the value should be set") { REQUIRE(array[3] == 15); }
 
 			THEN("the other values should be zero still")
 			{
-				for (ink::size_t i = 0; i < length; i++)
-				{
+				for (ink::size_t i = 0; i < length; i++) {
 					if (i == 3)
 						continue;
 
@@ -50,7 +45,7 @@ SCENARIO("a restorable array can save/restore/forget", "[array]")
 	GIVEN("a saved array with a few values")
 	{
 		// Load up the array
-		test_array array = test_array(5, 0, ~0);
+		test_array array = test_array(5, 0U, ~0U);
 		array.set(0, 0);
 		array.set(1, 1);
 		array.set(2, 2);

inkcpp_test/Callstack.cpp

@@ -58,7 +58,7 @@ SCENARIO("threading with the callstack", "[callstack]")
 
 				WHEN("we collapse to the main thraed")
 				{
-					stack.collapse_to_thread(~0);
+					stack.collapse_to_thread(~0U);
 
 					THEN("we should have the value from the original thread")
 					{
@@ -91,7 +91,7 @@ SCENARIO("threading with the callstack", "[callstack]")
 
 						THEN("we can still collapse to the main thread")
 						{
-							stack.collapse_to_thread(~0);
+							stack.collapse_to_thread(~0U);
 							REQUIRE(stack.get(X)->get<value_type::int32>() == 100);
 							REQUIRE(stack.get(Y)->get<value_type::int32>() == 200);
 						}
@@ -140,7 +140,7 @@ SCENARIO("threading with the callstack", "[callstack]")
 
 					WHEN("we collapse to the main thraed")
 					{
-						stack.collapse_to_thread(~0);
+						stack.collapse_to_thread(~0U);
 
 						THEN("we should have the value from the original thread")
 						{
@@ -214,7 +214,7 @@ SCENARIO("threading with the callstack", "[callstack]")
 
 				WHEN("we collapse back to the main thread")
 				{
-					stack.collapse_to_thread(~0);
+					stack.collapse_to_thread(~0U);
 					THEN("the stack should be inside the tunnel")
 					{
 						REQUIRE(stack.get(X)->type() == value_type::int32);
@@ -225,13 +225,13 @@ SCENARIO("threading with the callstack", "[callstack]")
 
 					WHEN("we do a tunnel return")
 					{
-						frame_type type;
-						auto       offset = stack.pop_frame(&type, eval_mode);
+						frame_type ftype;
+						auto       foffset = stack.pop_frame(&ftype, eval_mode);
 
 						THEN("we should be back outside")
 						{
-							REQUIRE(type == frame_type::tunnel);
-							REQUIRE(offset == 505);
+							REQUIRE(ftype == frame_type::tunnel);
+							REQUIRE(foffset == 505);
 							REQUIRE(stack.get(X)->type() == value_type::int32);
 							REQUIRE(stack.get(X)->get<value_type::int32>() == 100);
 							REQUIRE(stack.get(Y)->type() == value_type::int32);

inkcpp_test/FallbackFunction.cpp

@@ -22,7 +22,7 @@ SCENARIO("run a story with external function and fallback function", "[external
 			int  cnt_sqrt = 0;
 			auto fn_sqrt  = [&cnt_sqrt](int x) -> int {
         ++cnt_sqrt;
-        return sqrt(x);
+				return static_cast<int>(sqrt(x));
 			};
 			int  cnt_greeting = 0;
 			auto fn_greeting  = [&cnt_greeting]() -> const char* {
@@ -51,7 +51,7 @@ SCENARIO("run a story with external function and fallback function", "[external
 			int  cnt_sqrt = 0;
 			auto fn_sqrt  = [&cnt_sqrt](int x) -> int {
         ++cnt_sqrt;
-        return sqrt(x);
+				return static_cast<int>(sqrt(x));
 			};
 
 			thread->bind("sqrt", fn_sqrt);

inkcpp_test/Fixes.cpp

@@ -63,7 +63,8 @@ SCENARIO("unknown command _ #109", "[fixes]")
 			for (size_t i = 0; i < out_str.size(); ++i) {
 				data[i] = out_str[i];
 			}
-			std::unique_ptr<story> ink{story::from_binary(data, out_str.size())};
+			std::unique_ptr<story> ink{story::from_binary(data, static_cast<ink::size_t>(out_str.size()))
+			};
 			globals                globStore = ink->new_globals();
 			runner                 main      = ink->new_runner(globStore);
 			std::string            story     = main->getall();
@@ -158,17 +159,18 @@ SCENARIO(
 		{
 			thread->getall();
 			std::unique_ptr<snapshot> snap{thread->create_snapshot()};
-			runner                    thread = ink->new_runner_from_snapshot(*snap);
+			runner                    thread2 = ink->new_runner_from_snapshot(*snap);
 			const size_t s = reinterpret_cast<internal::snapshot_impl*>(snap.get())->strings().size();
 			THEN("loading it again will not change the string_table size")
 			{
-				runner       thread2 = ink->new_runner_from_snapshot(*snap);
+				runner       thread3 = ink->new_runner_from_snapshot(*snap);
 				const size_t s2 = reinterpret_cast<internal::snapshot_impl*>(snap.get())->strings().size();
 				REQUIRE(s == s2);
 			}
 		}
 	}
 }
+
 SCENARIO("Casting during redefinition is too strict _ #134", "[fixes]")
 {
 	GIVEN("story with problematic text")
@@ -226,9 +228,9 @@ SCENARIO("Using knot visit count as condition _ #139", "[fixes]")
 				WHEN("go to 'one' twice")
 				{
 					thread->choose(1);
-					std::string content = thread->getall();
+					std::string content2 = thread->getall();
 					REQUIRE(thread->num_choices() == 3);
-					THEN("get both one strings") { REQUIRE(content == "Check\nBeen here before\n"); }
+					THEN("get both one strings") { REQUIRE(content2 == "Check\nBeen here before\n"); }
 				}
 			}
 		}
@@ -248,4 +250,48 @@ SCENARIO("Using knot visit count as condition _ #139", "[fixes]")
 			}
 		}
 	}
-}
+}
+
+SCENARIO("Provoke thread array expension _ #142", "[fixes]")
+{
+	GIVEN("story with 15 threads in one know")
+	{
+		std::unique_ptr<story> ink{story::from_file(INK_TEST_RESOURCE_DIR "142_many_threads.bin")};
+		runner                 thread = ink->new_runner();
+		WHEN("just go to choice")
+		{
+			std::string content = thread->getall();
+			REQUIRE(content == "At the top\n");
+			THEN("expect to see 15 choices")
+			{
+				REQUIRE(thread->num_choices() == 15);
+				const char options[] = "abcdefghijklmno";
+				for (const char* c = options; *c; ++c) {
+					CHECK(thread->get_choice(static_cast<size_t>(c - options))->text()[0] == *c);
+				}
+			}
+		}
+		WHEN("choose 5 options")
+		{
+			std::string content = thread->getall();
+			for (int i = 0; i < 5; ++i) {
+				REQUIRE_FALSE(thread->can_continue());
+				thread->choose(i);
+				content += thread->getall();
+			}
+			REQUIRE(
+			    content
+			    == "At the top\na\nAt the top\nc\nAt the top\ne\nAt the top\ng\nAt the top\ni\nAt the "
+			       "top\n"
+			);
+			THEN("only 11 choices are left")
+			{
+				REQUIRE(thread->num_choices() == 10);
+				const char* options = "bdfhjklmno";
+				for (const char* c = options; *c; ++c) {
+					CHECK(thread->get_choice(static_cast<size_t>(c - options))->text()[0] == *c);
+				}
+			}
+		}
+	}
+}