Process: Rename StringsTable to StringsArena

Also this fixes a bug on the written StringView incorrectly containing extra null terminator

Author: Pagghiu

Libraries/Process/Internal/EnvironmentTable.h

@@ -1,7 +1,7 @@
 // Copyright (c) Stefano Cristiano
 // SPDX-License-Identifier: MIT
 #pragma once
-#include "StringsTable.h"
+#include "StringsArena.h"
 namespace SC
 {
 template <int MAX_NUM_ENVIRONMENT>
@@ -14,7 +14,7 @@ struct SC::EnvironmentTable
     using environment_ptr = const native_char_t* const*;
     const native_char_t* childEnvs[MAX_NUM_ENVIRONMENT];
 
-    Result writeTo(environment_ptr& environmentArray, bool inheritEnvironment, const StringsTable& table,
+    Result writeTo(environment_ptr& environmentArray, bool inheritEnvironment, const StringsArena& table,
                    const ProcessEnvironment& parentEnv)
     {
         if (table.bufferString.view().isEmpty())

Libraries/Process/Internal/ProcessPosix.inl

@@ -211,7 +211,7 @@ SC::Result SC::Process::launchImplementation()
 
         const char* const* environmentArray = environ;
         ProcessEnvironment parentEnv;
-        StringsTable       table = {environment, environmentNumber, environmentByteOffset};
+        StringsArena       table = {environment, environmentNumber, environmentByteOffset};
 
         EnvironmentTable<MAX_NUM_ENVIRONMENT> environmentTable;
         SC_TRY(environmentTable.writeTo(environmentArray, inheritEnv, table, parentEnv));
@@ -296,10 +296,10 @@ SC::Result SC::Process::launchImplementation()
 
 SC::Result SC::Process::formatArguments(Span<const StringView> params)
 {
-    StringsTable table = {command, commandArgumentsNumber, commandArgumentsByteOffset};
+    StringsArena table = {command, commandArgumentsNumber, commandArgumentsByteOffset};
     for (size_t idx = 0; idx < params.sizeInElements(); ++idx)
     {
-        SC_TRY(table.append(params[idx]));
+        SC_TRY(table.appendAsSingleString(params[idx]));
     }
     return Result(true);
 }

Libraries/Process/Internal/ProcessWindows.inl

@@ -87,20 +87,25 @@ SC::Result SC::Process::launchImplementation()
     LPCWSTR wideDir = currentDirectory.view().isEmpty() ? nullptr : currentDirectory.view().getNullTerminatedNative();
     LPWSTR  wideEnv = nullptr; // by default inherit parent environment
 
-    EnvironmentTable<MAX_NUM_ENVIRONMENT> environmenTable;
-    ProcessEnvironment                    parentEnv;
+    EnvironmentTable<MAX_NUM_ENVIRONMENT> environmentTable;
 
     const wchar_t* const* environmentArray = nullptr;
-    StringsTable          table            = {environment, environmentNumber, environmentByteOffset};
-    SC_TRY(environmenTable.writeTo(environmentArray, inheritEnv, table, parentEnv));
+
+    StringsArena arena = {environment, environmentNumber, environmentByteOffset};
+
+    ProcessEnvironment parentEnv;
+    SC_TRY(environmentTable.writeTo(environmentArray, inheritEnv, arena, parentEnv));
 
     if (environmentArray != nullptr)
     {
         for (size_t idx = environmentNumber; environmentArray[idx] != nullptr; ++idx)
         {
-            SC_TRY(table.append(StringView({environmentArray[idx], ::wcslen(environmentArray[idx])}, true)));
+            const StringView environmentString({environmentArray[idx], ::wcslen(environmentArray[idx])}, true);
+            SC_TRY(arena.appendAsSingleString(environmentString));
         }
-        SC_TRY(table.append({"\0"})); // add final \0
+        SC_TRY(arena.appendAsSingleString({"\0"})); // add final \0 (CreateProcessW requires it to signal end of array)
+
+        // const_cast is required by CreateProcessW signature unfortunately
         wideEnv = const_cast<LPWSTR>(environment.view().getNullTerminatedNative());
     }
     PROCESS_INFORMATION processInfo;

Libraries/Process/Internal/StringsTable.h renamed to Libraries/Process/Internal/StringsArena.h

@@ -4,18 +4,27 @@
 
 namespace SC
 {
-struct StringsTable;
+struct StringsArena;
 }
 
-/// @brief Appends a number of null-terminated StringView in a String keeping track of their starts
-/// Making this internal for now
-struct SC::StringsTable
+/// @brief Appends a variable number of null-terminated StringView in asingle String by keeping track of their starts
+struct SC::StringsArena
 {
-    String&      bufferString;
-    size_t&      numberOfStrings; // Counts number of arguments (including executable name)
-    Span<size_t> stringsStart;    // Tracking start of each string in the table
+    String&      bufferString;    // The underlying buffer / arena where strings are written to
+    size_t&      numberOfStrings; // Counts number of arguments
+    Span<size_t> stringsStart; // Tracking start of each string in the table, its size is used as max number of elements
 
-    [[nodiscard]] Result append(Span<const StringView> strings)
+    [[nodiscard]] Result appendMultipleStrings(Span<const StringView> strings)
+    {
+        for (const StringView string : strings)
+        {
+            SC_TRY(appendAsSingleString(string));
+        }
+        return Result(true);
+    }
+
+    // Appends a single string by joining multiple StringView together
+    [[nodiscard]] Result appendAsSingleString(Span<const StringView> strings)
     {
         if (numberOfStrings >= stringsStart.sizeInElements())
         {
@@ -25,7 +34,7 @@ struct SC::StringsTable
         stringsStart[numberOfStrings] = bufferString.sizeInBytesIncludingTerminator();
         for (size_t idx = 0; idx < strings.sizeInElements(); ++idx)
         {
-            SC_TRY(converter.appendNullTerminated(strings[idx], idx > 0)); // 0 == false
+            SC_TRY(converter.appendNullTerminated(strings[idx], idx > 0)); // idx > 0 == popExistingNullTerminator
         }
         numberOfStrings++;
         return Result(true);
@@ -38,12 +47,13 @@ struct SC::StringsTable
         {
             return Result::Error("StringTable::writeTo insufficient destination span");
         }
+        const auto sizeOfZero = StringEncodingGetSize(bufferString.getEncoding());
         for (size_t idx = 0; idx < numberOfStrings; ++idx)
         {
             const size_t lengthInBytes =
                 idx + 1 < numberOfStrings ? stringsStart[idx + 1] : bufferString.sizeInBytesIncludingTerminator();
-            strings[idx] = StringView({tableStart + stringsStart[idx], lengthInBytes - stringsStart[idx]}, true,
-                                      bufferString.getEncoding());
+            strings[idx] = StringView({tableStart + stringsStart[idx], lengthInBytes - stringsStart[idx] - sizeOfZero},
+                                      true, bufferString.getEncoding());
         }
         return Result(true);
     }

Libraries/Process/Process.cpp

@@ -231,6 +231,6 @@ SC::Result SC::Process::setWorkingDirectory(StringView processWorkingDirectory)
 
 SC::Result SC::Process::setEnvironment(StringView name, StringView value)
 {
-    StringsTable table = {environment, environmentNumber, environmentByteOffset};
-    return table.append({name, SC_NATIVE_STR("="), value});
+    StringsArena table = {environment, environmentNumber, environmentByteOffset};
+    return table.appendAsSingleString({name, SC_NATIVE_STR("="), value});
 }