Merge pull request #426 from jmittert/FreeFileContents

Handling Memory Across the API Boundary

Author: dmbryson

examples/c-api/buildsystem/main.c

@@ -22,6 +22,9 @@
 #include <string.h>
 
 #include <sys/stat.h>
+#if defined(_WIN32)
+#include <windows.h>
+#endif
 
 static const char* progname;
 static void usage() {
@@ -34,6 +37,27 @@ static const char* basename(const char* path) {
   return result ? result : path;
 }
 
+#if defined(_WIN32)
+static wchar_t* convertToMultiByte(const char* s) {
+  int numChars = MultiByteToWideChar(
+      /*CodePage=*/CP_UTF8,
+      /*dwFlags=*/0,
+      /*lpMultibyteStr=*/s,
+      /*cbMultiByte=*/-1,
+      /*lpWideCharStr=*/NULL,
+      /*ccWideChar=*/0);
+  wchar_t* mbs = malloc(numChars * sizeof(wchar_t));
+  MultiByteToWideChar(
+      /*CodePage=*/CP_UTF8,
+      /*dwFlags=*/0,
+      /*lpMultibyteStr=*/s,
+      /*cbMultiByte=*/-1,
+      /*lpWideCharStr=*/mbs,
+      /*ccWideChar=*/numChars);
+  return mbs;
+}
+#endif
+
 // "Fancy" Command Implementation
 
 static bool
@@ -61,18 +85,30 @@ fancy_tool_create_command(void *context, const llb_data_t* name) {
 
 static bool fs_get_file_contents(void* context, const char* path,
                                  llb_data_t* data_out) {
+
+#if defined(_WIN32)
+  wchar_t* wPath = convertToMultiByte(path);
+  wprintf(L" -- read file contents: %ls\n", wPath);
+  fflush(stdout);
+  FILE* fp;
+  if (_wfopen_s(&fp, wPath, L"rb")) {
+    free(wPath);
+    return false;
+  }
+  free(wPath);
+#else
   printf(" -- read file contents: %s\n", path);
   fflush(stdout);
-
   FILE *fp = fopen(path, "rb");
   if (!fp) {
     return false;
   }
-  
+#endif
+
   fseek(fp, 0, SEEK_END);
   long size = ftell(fp);
   fseek(fp, 0, SEEK_SET);
-  uint8_t* buffer = malloc(size);
+  uint8_t* buffer = (uint8_t*)llb_alloc(size);
   if (!buffer) {
     return false;
   }
@@ -107,7 +143,7 @@ static void fs_get_file_info(void* context, const char* path,
 static llb_buildsystem_tool_t* lookup_tool(void *context,
                                            const llb_data_t* name) {
   if (name->length == 5 && memcmp(name->data, "fancy", 5) == 0) {
-    llb_buildsystem_tool_delegate_t delegate = {};
+    llb_buildsystem_tool_delegate_t delegate = {0};
     delegate.create_command = fancy_tool_create_command;
     return llb_buildsystem_tool_create(name, delegate);
   }
@@ -136,7 +172,7 @@ static void command_started(void* context,
   llb_buildsystem_command_get_name(command, &name);
   printf("%s: %.*s -- %s\n", __FUNCTION__, (int)name.length, name.data,
          description);
-  free(description);
+  llb_free(description);
   fflush(stdout);
 }
 
@@ -192,12 +228,12 @@ int main(int argc, char **argv) {
   const char* buildFilePath = argv[1];
 
   // Create an invocation.
-  llb_buildsystem_invocation_t invocation = {};
+  llb_buildsystem_invocation_t invocation = {0};
   invocation.buildFilePath = buildFilePath;
   invocation.useSerialBuild = true;
 
   // Create a build system delegate.
-  llb_buildsystem_delegate_t delegate = {};
+  llb_buildsystem_delegate_t delegate = {0};
   delegate.context = NULL;
   delegate.fs_get_file_contents = fs_get_file_contents;
   delegate.fs_get_file_info = fs_get_file_info;

lib/Basic/Subprocess.cpp

@@ -416,7 +416,9 @@ void llbuild::basic::spawnProcess(
   // gets pulled in.
   std::for_each(std::begin(argsStorage) + 1, std::end(argsStorage),
                 [&args](auto arg) { args += arg + " "; });
-  args.pop_back();
+  if (!args.empty()) {
+    args.pop_back();
+  }
   // Convert the command line string to utf16
   llvm::SmallVector<UTF16, 20> u16Executable;
   llvm::SmallVector<UTF16, 20> u16CmdLine;
@@ -603,6 +605,7 @@ void llbuild::basic::spawnProcess(
     if (!res.getError()) {
       argsStorage[0] = *res;
 #if defined(_WIN32)
+      u16Executable.clear();
       llvm::convertUTF8ToUTF16String(argsStorage[0], u16Executable);
 #else
       args[0] = argsStorage[0].c_str();

products/libllbuild/BuildSystem-C-API.cpp

@@ -932,3 +932,5 @@ void llb_set_quality_of_service(llb_quality_of_service_t level) {
   }
 }
 
+void* llb_alloc(size_t size) { return malloc(size); }
+void llb_free(void* ptr) { free(ptr); }

products/libllbuild/include/llbuild/buildsystem.h

@@ -254,11 +254,11 @@ typedef struct llb_buildsystem_delegate_t_ {
   ///
   /// \\returns True on success (the directory was created, or already exists).
   bool (*fs_create_directory)(void* context, const char* path);
-  
+
   /// Get the file contents for the given path.
   ///
   /// The contents *MUST* be returned in a new buffer allocated with \see
-  /// malloc().
+  /// llb_alloc().
   ///
   /// Xparam path The path to provide the contents for.
   ///
@@ -735,7 +735,18 @@ llb_get_scheduler_lane_width();
 /// be automatically translated into the number of cores detected on the host.
 LLBUILD_EXPORT void
 llb_set_scheduler_lane_width(uint32_t width);
+/// @}
 
+/// @name Memory APIs
+// MARK: Allocating and freeing memory
+/// Allocate memory usable by the build system
+/// \param size The number bytes to allocate
+LLBUILD_EXPORT void*
+llb_alloc(size_t size);
+/// Free memory allocated for or by the build system
+/// \param ptr A pointer to the allocated memory to free
+LLBUILD_EXPORT void
+llb_free(void* ptr);
 /// @}
 
 #endif

tests/Examples/buildsystem-capi.llbuild

@@ -4,7 +4,7 @@
 # RUN: env LD_LIBRARY_PATH=%{llbuild-lib-dir} %t.exe %s > %t.out
 # RUN: %{FileCheck} %s --input-file %t.out
 #
-# CHECK: -- read file contents: {{.*}}/buildsystem-capi.llbuild
+# CHECK: -- read file contents: {{.*[/\\]}}buildsystem-capi.llbuild
 # CHECK: initial build:
 # CHECK: -- stat: /
 # CHECK: command_started: <hello> -- HELLO

unittests/CAPI/BuildSystem-C-API.cpp

@@ -10,9 +10,10 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "llbuild/llbuild.h"
 #include "llbuild/Basic/PlatformUtility.h"
+#include "llbuild/llbuild.h"
 #include "llbuild/buildsystem.h"
+#include "llvm/Support/ConvertUTF.h"
 #include <stdlib.h>
 #include <string.h>
 
@@ -119,7 +120,7 @@ depinfo_tester_command_execute_command(void *context,
   writeFileContents(depInfoPath, depInfoContents);
 
   // Clean up.
-  free(desc);
+  llb_free(desc);
   return true;
 }
 
@@ -134,18 +135,29 @@ depinfo_tester_tool_create_command(void *context, const llb_data_t* name) {
 
 static bool fs_get_file_contents(void* context, const char* path,
                                 llb_data_t* data_out) {
+#if defined(_WIN32)
+  llvm::SmallVector<UTF16, 20> wPath;
+  llvm::convertUTF8ToUTF16String(path, wPath);
+  wprintf(L" -- read file contents: %ls\n", (LPCWSTR)wPath.data());
+  fflush(stdout);
+  FILE* fp;
+  if (_wfopen_s(&fp, (LPCWSTR)wPath.data(), L"rb")) {
+    return false;
+  }
+#else
   printf(" -- read file contents: %s\n", path);
   fflush(stdout);
-  
+
   FILE *fp = fopen(path, "rb");
   if (!fp) {
     return false;
   }
-  
+#endif
+
   fseek(fp, 0, SEEK_END);
   long size = ftell(fp);
   fseek(fp, 0, SEEK_SET);
-  uint8_t* buffer = (uint8_t *)malloc(size);
+  uint8_t* buffer = (uint8_t*)llb_alloc(size);
   if (!buffer) {
     return false;
   }
@@ -209,7 +221,7 @@ static void command_started(void* context,
   llb_buildsystem_command_get_name(command, &name);
   printf("%s: %.*s -- %s\n", __FUNCTION__, (int)name.length, name.data,
          description);
-  free(description);
+  llb_free(description);
   fflush(stdout);
 }