Change C APIs to a unified string implementation (#6299)

Currently in the entire C API of WPILib we have ~8 different ways of handling strings. The C API actually isn't built for pure C callers (We don't actually have any of those). Instead, they're built for interop between languages like LabVIEW and C# which can talk to C API's directly.

For output parameters, the choice was fairly obvious. An output struct containing a const string pointer and a length makes the most sense. Its easy to use these from most other languages, and doesn't require special null termination handling. Freeing these is also easy, as if you ever receive one of these string structures, theres just a single function call to free it.

Input parameters are a bit more complex. To be used from pure C, and from LabVIEW, a null terminated string is the best in most cases. However, null terminated strings in general have a lot of downsides. Additionally, from LabVIEW there are other considerations around encoding that having a wrapper struct helps make a bit easier. From a language like C#, a wrapper struct is by far the easiest, as custom marshalling can make it trivial to marshal both UTF8 and UTF16 strings down.

The final consideration is its nice to have an identical concept for both input and output. It makes the rules fairly easy to understand.

WPILib will not have any APIs that manipulate a string allocated externally. This means WPI_String can be const, as across the boundary it is always const.
If a WPILib API takes a const WPI_String*, WPILib will not manipulate or attempt to free that string, and that string is treated as an input. It is up to the caller to handle that memory, WPILib will never hold onto that memory longer than the call.
If a WPILib API takes a WPI_String*, that string is an output. WPILib will allocate that API with WPI_AllocateString(), fill in the string, and return to the caller. When the caller is done with the string, they must free it with WPI_FreeString().
If an output struct contains a WPI_String member, that member is considered read only, and should not be explicitly freed. The caller should call the free function for that struct.
If an array of WPI_Strings are returned, each individual string is considered read only, and should not be explicitly freed. The free function for that array should be called by the caller.
If an input struct containing a WPI_String, or an input array of WPI_Strings is passed to WPILib, the individual strings will not be manipulated or freed by WPILib, and the caller owns and should free that memory.
Callbacks also follow these rules. The most common is a callback either getting passed a const WPI_String* or a struct containing a WPI_String. In both of these cases, the callback target should consider these strings read only, and not attempt to free them or manipulate them.

wpiutil/src/main/native/cpp/DataLog.cpp

@@ -596,7 +596,7 @@ void DataLog::AppendStringArray(int entry,
 }
 
 void DataLog::AppendStringArray(int entry,
-                                std::span<const WPI_DataLog_String> arr,
+                                std::span<const struct WPI_String> arr,
                                 int64_t timestamp) {
   if (entry <= 0) {
     return;
@@ -640,11 +640,13 @@ void WPI_DataLog_Stop(struct WPI_DataLog* datalog) {
   reinterpret_cast<DataLog*>(datalog)->Stop();
 }
 
-int WPI_DataLog_Start(struct WPI_DataLog* datalog, const char* name,
-                      const char* type, const char* metadata,
-                      int64_t timestamp) {
-  return reinterpret_cast<DataLog*>(datalog)->Start(name, type, metadata,
-                                                    timestamp);
+int WPI_DataLog_Start(struct WPI_DataLog* datalog,
+                      const struct WPI_String* name,
+                      const struct WPI_String* type,
+                      const struct WPI_String* metadata, int64_t timestamp) {
+  return reinterpret_cast<DataLog*>(datalog)->Start(
+      wpi::to_string_view(name), wpi::to_string_view(type),
+      wpi::to_string_view(metadata), timestamp);
 }
 
 void WPI_DataLog_Finish(struct WPI_DataLog* datalog, int entry,
@@ -653,8 +655,10 @@ void WPI_DataLog_Finish(struct WPI_DataLog* datalog, int entry,
 }
 
 void WPI_DataLog_SetMetadata(struct WPI_DataLog* datalog, int entry,
-                             const char* metadata, int64_t timestamp) {
-  reinterpret_cast<DataLog*>(datalog)->SetMetadata(entry, metadata, timestamp);
+                             const struct WPI_String* metadata,
+                             int64_t timestamp) {
+  reinterpret_cast<DataLog*>(datalog)->SetMetadata(
+      entry, wpi::to_string_view(metadata), timestamp);
 }
 
 void WPI_DataLog_AppendRaw(struct WPI_DataLog* datalog, int entry,
@@ -683,10 +687,10 @@ void WPI_DataLog_AppendDouble(struct WPI_DataLog* datalog, int entry,
 }
 
 void WPI_DataLog_AppendString(struct WPI_DataLog* datalog, int entry,
-                              const char* value, size_t len,
+                              const struct WPI_String* value,
                               int64_t timestamp) {
-  reinterpret_cast<DataLog*>(datalog)->AppendString(entry, {value, len},
-                                                    timestamp);
+  reinterpret_cast<DataLog*>(datalog)->AppendString(
+      entry, {value->str, value->len}, timestamp);
 }
 
 void WPI_DataLog_AppendBooleanArray(struct WPI_DataLog* datalog, int entry,
@@ -725,23 +729,29 @@ void WPI_DataLog_AppendDoubleArray(struct WPI_DataLog* datalog, int entry,
 }
 
 void WPI_DataLog_AppendStringArray(struct WPI_DataLog* datalog, int entry,
-                                   const WPI_DataLog_String* arr, size_t len,
+                                   const struct WPI_String* arr, size_t len,
                                    int64_t timestamp) {
   reinterpret_cast<DataLog*>(datalog)->AppendStringArray(entry, {arr, len},
                                                          timestamp);
 }
 
-void WPI_DataLog_AddSchemaString(struct WPI_DataLog* datalog, const char* name,
-                                 const char* type, const char* schema,
+void WPI_DataLog_AddSchemaString(struct WPI_DataLog* datalog,
+                                 const struct WPI_String* name,
+                                 const struct WPI_String* type,
+                                 const struct WPI_String* schema,
                                  int64_t timestamp) {
-  reinterpret_cast<DataLog*>(datalog)->AddSchema(name, type, schema, timestamp);
+  reinterpret_cast<DataLog*>(datalog)->AddSchema(
+      wpi::to_string_view(name), wpi::to_string_view(type),
+      wpi::to_string_view(schema), timestamp);
 }
 
-void WPI_DataLog_AddSchema(struct WPI_DataLog* datalog, const char* name,
-                           const char* type, const uint8_t* schema,
+void WPI_DataLog_AddSchema(struct WPI_DataLog* datalog,
+                           const struct WPI_String* name,
+                           const struct WPI_String* type, const uint8_t* schema,
                            size_t schema_len, int64_t timestamp) {
   reinterpret_cast<DataLog*>(datalog)->AddSchema(
-      name, type, std::span<const uint8_t>{schema, schema_len}, timestamp);
+      wpi::to_string_view(name), wpi::to_string_view(type),
+      std::span<const uint8_t>{schema, schema_len}, timestamp);
 }
 
 }  // extern "C"

wpiutil/src/main/native/cpp/DataLogBackgroundWriter.cpp

@@ -461,23 +461,25 @@ void DataLogBackgroundWriter::WriterThreadMain(
 extern "C" {
 
 struct WPI_DataLog* WPI_DataLog_CreateBackgroundWriter(
-    const char* dir, const char* filename, double period,
-    const char* extraHeader) {
-  return reinterpret_cast<WPI_DataLog*>(
-      new DataLogBackgroundWriter{dir, filename, period, extraHeader});
+    const struct WPI_String* dir, const struct WPI_String* filename,
+    double period, const struct WPI_String* extraHeader) {
+  return reinterpret_cast<WPI_DataLog*>(new DataLogBackgroundWriter{
+      wpi::to_string_view(dir), wpi::to_string_view(filename), period,
+      wpi::to_string_view(extraHeader)});
 }
 
 struct WPI_DataLog* WPI_DataLog_CreateBackgroundWriter_Func(
     void (*write)(void* ptr, const uint8_t* data, size_t len), void* ptr,
-    double period, const char* extraHeader) {
+    double period, const struct WPI_String* extraHeader) {
   return reinterpret_cast<WPI_DataLog*>(new DataLogBackgroundWriter{
       [=](auto data) { write(ptr, data.data(), data.size()); }, period,
-      extraHeader});
+      wpi::to_string_view(extraHeader)});
 }
 
-void WPI_DataLog_SetBackgroundWriterFilename(struct WPI_DataLog* datalog,
-                                             const char* filename) {
-  reinterpret_cast<DataLogBackgroundWriter*>(datalog)->SetFilename(filename);
+void WPI_DataLog_SetBackgroundWriterFilename(
+    struct WPI_DataLog* datalog, const struct WPI_String* filename) {
+  reinterpret_cast<DataLogBackgroundWriter*>(datalog)->SetFilename(
+      wpi::to_string_view(filename));
 }
 
 }  // extern "C"

wpiutil/src/main/native/cpp/DataLogWriter.cpp

@@ -63,12 +63,12 @@ bool DataLogWriter::BufferFull() {
 
 extern "C" {
 
-struct WPI_DataLog* WPI_DataLog_CreateWriter(const char* filename,
-                                             int* errorCode,
-                                             const char* extraHeader) {
+struct WPI_DataLog* WPI_DataLog_CreateWriter(
+    const struct WPI_String* filename, int* errorCode,
+    const struct WPI_String* extraHeader) {
   std::error_code ec;
-  auto rv = reinterpret_cast<WPI_DataLog*>(
-      new DataLogWriter{filename, ec, extraHeader});
+  auto rv = reinterpret_cast<WPI_DataLog*>(new DataLogWriter{
+      wpi::to_string_view(filename), ec, wpi::to_string_view(extraHeader)});
   *errorCode = ec.value();
   return rv;
 }

wpiutil/src/main/native/cpp/string.cpp

@@ -0,0 +1,77 @@
+// Copyright (c) FIRST and other WPILib contributors.
+// Open Source Software; you can modify and/or share it under the terms of
+// the WPILib BSD license file in the root directory of this project.
+
+#include "wpi/string.h"
+
+#include <wpi/MemAlloc.h>
+
+#include <string_view>
+
+extern "C" {
+
+void WPI_InitString(struct WPI_String* wpiString, const char* utf8String) {
+  if (wpiString == nullptr) {
+    return;
+  }
+  if (utf8String == nullptr) {
+    wpiString->str = nullptr;
+    wpiString->len = 0;
+  } else {
+    wpiString->str = utf8String;
+    wpiString->len = std::char_traits<char>::length(utf8String);
+  }
+}
+
+void WPI_InitStringWithLength(struct WPI_String* wpiString,
+                              const char* utf8String, size_t length) {
+  if (wpiString == nullptr) {
+    return;
+  }
+  wpiString->str = utf8String;
+  wpiString->len = length;
+}
+
+// Returned from AllocateString if 0 length is requested.
+// Returned instead of nullptr due to memcpy pointer validity rules
+static char writeBuffer;
+
+char* WPI_AllocateString(struct WPI_String* wpiString, size_t length) {
+  if (wpiString == nullptr) {
+    return nullptr;
+  }
+  if (length == 0) {
+    wpiString->len = 0;
+    wpiString->str = nullptr;
+    return &writeBuffer;
+  }
+  char* str = static_cast<char*>(wpi::safe_malloc(length));
+  wpiString->str = str;
+  wpiString->len = length;
+  return str;
+}
+
+void WPI_FreeString(const struct WPI_String* wpiString) {
+  if (wpiString == nullptr) {
+    return;
+  }
+  std::free(const_cast<char*>(wpiString->str));
+}
+
+struct WPI_String* WPI_AllocateStringArray(size_t length) {
+  return static_cast<struct WPI_String*>(
+      wpi::safe_malloc(length * sizeof(struct WPI_String)));
+}
+
+void WPI_FreeStringArray(const struct WPI_String* wpiStringArray,
+                         size_t length) {
+  if (wpiStringArray == nullptr) {
+    return;
+  }
+  for (size_t i = 0; i < length; ++i) {
+    WPI_FreeString(&wpiStringArray[i]);
+  }
+  std::free(const_cast<struct WPI_String*>(wpiStringArray));
+}
+
+}  // extern "C"

wpiutil/src/main/native/include/wpi/DataLog.h

@@ -23,6 +23,7 @@
 #include "wpi/StringMap.h"
 #include "wpi/mutex.h"
 #include "wpi/protobuf/Protobuf.h"
+#include "wpi/string.h"
 #include "wpi/struct/Struct.h"
 #include "wpi/timestamp.h"
 
@@ -373,7 +374,7 @@ class DataLog {
    * @param arr String array to record
    * @param timestamp Time stamp (may be 0 to indicate now)
    */
-  void AppendStringArray(int entry, std::span<const WPI_DataLog_String> arr,
+  void AppendStringArray(int entry, std::span<const struct WPI_String> arr,
                          int64_t timestamp);
 
  protected:

wpiutil/src/main/native/include/wpi/DataLog_c.h

@@ -7,22 +7,12 @@
 #include <stddef.h>  // NOLINT
 
 #include <stdint.h>
+#include <wpi/string.h>
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
-/**
- * A datalog string (for use with string array).
- */
-struct WPI_DataLog_String {
-  /** Contents. */
-  const char* str;
-
-  /** Length. */
-  size_t len;
-};
-
 /** C-compatible data log (opaque struct). */
 struct WPI_DataLog;
 
@@ -33,9 +23,9 @@ struct WPI_DataLog;
  * @param errorCode error if file failed to open (output)
  * @param extraHeader extra header data
  */
-struct WPI_DataLog* WPI_DataLog_CreateWriter(const char* filename,
-                                             int* errorCode,
-                                             const char* extraHeader);
+struct WPI_DataLog* WPI_DataLog_CreateWriter(
+    const struct WPI_String* filename, int* errorCode,
+    const struct WPI_String* extraHeader);
 
 /**
  * Construct a new Data Log background writer.  The log will be initially
@@ -48,10 +38,9 @@ struct WPI_DataLog* WPI_DataLog_CreateWriter(const char* filename,
  *               this is a time/storage tradeoff
  * @param extraHeader extra header data
  */
-struct WPI_DataLog* WPI_DataLog_CreateBackgroundWriter(const char* dir,
-                                                       const char* filename,
-                                                       double period,
-                                                       const char* extraHeader);
+struct WPI_DataLog* WPI_DataLog_CreateBackgroundWriter(
+    const struct WPI_String* dir, const struct WPI_String* filename,
+    double period, const struct WPI_String* extraHeader);
 
 /**
  * Construct a new Data Log background writer that passes its output to the
@@ -67,7 +56,7 @@ struct WPI_DataLog* WPI_DataLog_CreateBackgroundWriter(const char* dir,
  */
 struct WPI_DataLog* WPI_DataLog_CreateBackgroundWriter_Func(
     void (*write)(void* ptr, const uint8_t* data, size_t len), void* ptr,
-    double period, const char* extraHeader);
+    double period, const struct WPI_String* extraHeader);
 
 /**
  * Change log filename.  Can only be used on background writer data logs.
@@ -76,7 +65,7 @@ struct WPI_DataLog* WPI_DataLog_CreateBackgroundWriter_Func(
  * @param filename filename
  */
 void WPI_DataLog_SetBackgroundWriterFilename(struct WPI_DataLog* datalog,
-                                             const char* filename);
+                                             const struct WPI_String* filename);
 
 /**
  * Releases a data log object. Closes the file and returns resources to the
@@ -134,9 +123,10 @@ void WPI_DataLog_Stop(struct WPI_DataLog* datalog);
  *
  * @return Entry index
  */
-int WPI_DataLog_Start(struct WPI_DataLog* datalog, const char* name,
-                      const char* type, const char* metadata,
-                      int64_t timestamp);
+int WPI_DataLog_Start(struct WPI_DataLog* datalog,
+                      const struct WPI_String* name,
+                      const struct WPI_String* type,
+                      const struct WPI_String* metadata, int64_t timestamp);
 
 /**
  * Finish an entry.
@@ -157,7 +147,8 @@ void WPI_DataLog_Finish(struct WPI_DataLog* datalog, int entry,
  * @param timestamp Time stamp (may be 0 to indicate now)
  */
 void WPI_DataLog_SetMetadata(struct WPI_DataLog* datalog, int entry,
-                             const char* metadata, int64_t timestamp);
+                             const struct WPI_String* metadata,
+                             int64_t timestamp);
 
 /**
  * Appends a raw record to the log.
@@ -221,11 +212,11 @@ void WPI_DataLog_AppendDouble(struct WPI_DataLog* datalog, int entry,
  * @param datalog data log
  * @param entry Entry index, as returned by WPI_DataLog_Start()
  * @param value String value to record
- * @param len Length of string
  * @param timestamp Time stamp (may be 0 to indicate now)
  */
 void WPI_DataLog_AppendString(struct WPI_DataLog* datalog, int entry,
-                              const char* value, size_t len, int64_t timestamp);
+                              const struct WPI_String* value,
+                              int64_t timestamp);
 
 /**
  * Appends a boolean array record to the log.
@@ -302,15 +293,18 @@ void WPI_DataLog_AppendDoubleArray(struct WPI_DataLog* datalog, int entry,
  * @param timestamp Time stamp (may be 0 to indicate now)
  */
 void WPI_DataLog_AppendStringArray(struct WPI_DataLog* datalog, int entry,
-                                   const WPI_DataLog_String* arr, size_t len,
+                                   const struct WPI_String* arr, size_t len,
                                    int64_t timestamp);
 
-void WPI_DataLog_AddSchemaString(struct WPI_DataLog* datalog, const char* name,
-                                 const char* type, const char* schema,
+void WPI_DataLog_AddSchemaString(struct WPI_DataLog* datalog,
+                                 const struct WPI_String* name,
+                                 const struct WPI_String* type,
+                                 const struct WPI_String* schema,
                                  int64_t timestamp);
 
-void WPI_DataLog_AddSchema(struct WPI_DataLog* datalog, const char* name,
-                           const char* type, const uint8_t* schema,
+void WPI_DataLog_AddSchema(struct WPI_DataLog* datalog,
+                           const struct WPI_String* name,
+                           const struct WPI_String* type, const uint8_t* schema,
                            size_t schema_len, int64_t timestamp);
 
 #ifdef __cplusplus

wpiutil/src/main/native/include/wpi/jni_util.h

@@ -23,6 +23,7 @@
 #include "wpi/mutex.h"
 #include "wpi/print.h"
 #include "wpi/raw_ostream.h"
+#include "wpi/string.h"
 
 /** Java Native Interface (JNI) utility functions */
 namespace wpi::java {
@@ -171,6 +172,7 @@ class JStringRef {
   std::string_view str() const { return m_str.str(); }
   const char* c_str() const { return m_str.data(); }
   size_t size() const { return m_str.size(); }
+  WPI_String wpi_str() const { return wpi::make_string(str()); }
 
  private:
   SmallString<128> m_str;

wpiutil/src/main/native/include/wpi/string.h

@@ -0,0 +1,108 @@
+// Copyright (c) FIRST and other WPILib contributors.
+// Open Source Software; you can modify and/or share it under the terms of
+// the WPILib BSD license file in the root directory of this project.
+
+#pragma once
+
+#ifdef __cplusplus
+#include <string_view>
+#endif
+
+/**
+ * A const UTF8 string.
+ */
+struct WPI_String {
+  /** Contents. */
+  const char* str;
+  /** Length */
+  size_t len;
+};
+
+#ifdef __cplusplus
+namespace wpi {
+/** Converts a WPI_String to a string_view */
+constexpr std::string_view to_string_view(const struct WPI_String* str) {
+  if (str) {
+    return {str->str, str->len};
+  } else {
+    return "";
+  }
+}
+
+/** Converts a string_view to a WPI_String */
+constexpr WPI_String make_string(std::string_view view) {
+  return WPI_String{view.data(), view.size()};
+}
+}  // namespace wpi
+#endif  // __cplusplus
+
+#ifdef __cplusplus
+extern "C" {
+#endif  // __cplusplus
+
+/**
+ * Initializes a WPI_String from a null terminated UTF-8 string.
+ * If input string is null, initializes output to 0 length.
+ * The output length does not include the null terminator.
+ *
+ * The lifetime of the output string is the lifetime of the input string.
+ * Do not call WPI_FreeString() with the output of this call.
+ *
+ * @param wpiString output string
+ * @param utf8String input string (null terminated)
+ */
+void WPI_InitString(struct WPI_String* wpiString, const char* utf8String);
+
+/**
+ * Initializes a WPI_String from a UTF-8 string and length.
+ * If input string is null or 0 length, initilizes output to 0 length.
+ * The input string does not need to be null terminated.
+ *
+ * The lifetime of the output string is the lifetime of the input string.
+ * Do not call WPI_FreeString() with the output of this call.
+ *
+ * @param wpiString output string
+ * @param utf8String input string
+ * @param length input string length in chars
+ */
+void WPI_InitStringWithLength(struct WPI_String* wpiString,
+                              const char* utf8String, size_t length);
+
+/**
+ * Allocates a WPI_String for the specified length.
+ * The resultant string must be freed with WPI_FreeString().
+ *
+ * @param wpiString output string
+ * @param length string length in chars to allocate
+ * @return mutable pointer to allocated buffer
+ *
+ */
+char* WPI_AllocateString(struct WPI_String* wpiString, size_t length);
+
+/**
+ * Frees a WPI_String that was allocated with WPI_AllocateString()
+ *
+ * @param wpiString string to free
+ */
+void WPI_FreeString(const struct WPI_String* wpiString);
+
+/**
+ * Allocates an array of WPI_Strings.
+ *
+ * @param length array length
+ * @return string array
+ */
+struct WPI_String* WPI_AllocateStringArray(size_t length);
+
+/**
+ * Frees a WPI_String array returned by WPI_AllocateStringArray().
+ *
+ * @param wpiStringArray string array to free
+ * @param length length of array
+ */
+void WPI_FreeStringArray(const struct WPI_String* wpiStringArray,
+                         size_t length);
+
+#ifdef __cplusplus
+}  // extern "C"
+#endif  // __cplusplus