refactor(cpn): introduce BoundedString<N> for ModelData string fields

.github/workflows/companion.yml

@@ -119,6 +119,10 @@ jobs:
         with:
           os: 'linux'
 
+      - name: Run companion unit tests
+        shell: bash
+        run: cmake --build build/native --target tests-companion --parallel
+
   build-macos:
     name: macOS Companion
     runs-on: macos-15

companion/src/firmwares/boundedstring.h

@@ -0,0 +1,92 @@
+/*
+ * Copyright (C) EdgeTX
+ *
+ * Based on code named
+ *   opentx - https://github.com/opentx/opentx
+ *   th9x - http://code.google.com/p/th9x
+ *   er9x - http://code.google.com/p/er9x
+ *   gruvin9x - http://code.google.com/p/gruvin9x
+ *
+ * License GPLv2: http://www.gnu.org/licenses/gpl-2.0.html
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#pragma once
+
+#include <string>
+#include <string_view>
+
+#include <QString>
+
+// A self-limiting string for Companion's data-model fields.
+//
+// N is the maximum number of bytes the value may hold, EXCLUDING the implicit
+// NUL of the legacy `char[]` fields it replaces. So a legacy `char foo[16+1]`
+// (16 usable chars) maps to `BoundedString<16>`.
+//
+// Assignment always truncates to N bytes, mirroring the byte-level truncation
+// the YAML char-array reader does today (yaml_ops.h: `str.copy(value, N-1)`).
+// Truncation is byte-based on purpose: it must match the radio firmware (and the
+// legacy code path) exactly, including the case where a multi-byte UTF-8
+// sequence is split at the boundary. Do not "improve" it to be codepoint-aware.
+template <size_t N>
+class BoundedString
+{
+  static_assert(N > 0, "BoundedString capacity must be greater than zero");
+
+  std::string s_;
+
+ public:
+  static constexpr size_t capacity() { return N; }
+
+  BoundedString() = default;
+  BoundedString(const BoundedString&) = default;
+  BoundedString& operator=(const BoundedString&) = default;
+
+  BoundedString(std::string_view v) { assign(v); }
+  BoundedString(const char* v) { assign(v ? std::string_view(v) : std::string_view()); }
+  BoundedString(const QString& v) { assign(v); }
+
+  BoundedString& operator=(std::string_view v) { assign(v); return *this; }
+  BoundedString& operator=(const char* v) { assign(v ? std::string_view(v) : std::string_view()); return *this; }
+  BoundedString& operator=(const QString& v) { assign(v); return *this; }
+
+  void assign(std::string_view v) { s_.assign(v.substr(0, N)); }
+  // NOTE: QString is serialised as UTF-8 here, matching the YAML layer. Some
+  // legacy UI call sites stored Latin-1 via QString::toLatin1(); Phase 1 must
+  // confirm the encoding at each migrated boundary before relying on this.
+  void assign(const QString& v) { assign(v.toStdString()); }
+
+  void clear() { s_.clear(); }
+  bool empty() const { return s_.empty(); }
+  size_t size() const { return s_.size(); }
+  size_t length() const { return s_.size(); }
+
+  const std::string& str() const { return s_; }
+  const char* c_str() const { return s_.c_str(); }
+  QString toQString() const { return QString::fromStdString(s_); }
+
+  // Implicit conversion so a migrated field still drops into APIs taking a
+  // std::string (and, via QString's std::string ctor, much of the Qt UI).
+  operator const std::string&() const { return s_; }
+
+  // Explicit comparison overloads (std::string's own operator== is a
+  // non-member template, so the implicit conversion above can't drive it).
+  // The const char* overload binds legacy char[]/literals via array-to-pointer
+  // (a standard conversion), so it wins cleanly over the others rather than
+  // tying with them and producing an ambiguity.
+  bool operator==(const BoundedString& o) const { return s_ == o.s_; }
+  bool operator!=(const BoundedString& o) const { return s_ != o.s_; }
+  bool operator==(const std::string& o) const { return s_ == o; }
+  bool operator!=(const std::string& o) const { return s_ != o; }
+  bool operator==(const char* o) const { return s_ == o; }
+  bool operator!=(const char* o) const { return s_ != o; }
+};

companion/src/firmwares/edgetx/yaml_modeldata.cpp

@@ -46,17 +46,17 @@
 
 void YamlValidateLabelsNames(ModelData& model, Board::Type board)
 {
-  YamlValidateName(model.name, board);
+  model.name = YamlValidateName(model.name.toQString(), board).toLatin1().constData();
 
-  QStringList lst = QString(model.labels).split(',', Qt::SkipEmptyParts);
+  QStringList lst = model.labels.toQString().split(',', Qt::SkipEmptyParts);
 
   for (int i = lst.count() - 1; i >= 0; i--) {
     YamlValidateLabel(lst[i]);
     if (lst.at(i).isEmpty())
       lst.removeAt(i);
   }
 
-  strcpy(model.labels, QString(lst.join(',')).toLatin1().data());
+  model.labels = QString(lst.join(',')).toLatin1().constData();
 
   for (int i = 0; i < CPN_MAX_CURVES; i++) {
     YamlValidateName(model.curves[i].name, board);
@@ -418,7 +418,7 @@ struct YamlSwitchWarningState {
     std::stringstream ss(src_str);
     while (!ss.eof()) {
       auto c = ss.get();
-      if (c < 'A' && c > 'Z') {
+      if (c < 'A' || c > 'Z') {
         ss.ignore();
         continue;
       }
@@ -1324,12 +1324,12 @@ bool convert<ModelData>::decode(const Node& node, ModelData& rhs)
 
   if (node["semver"]) {
     node["semver"] >> rhs.semver;
-    if (SemanticVersion().isValid(rhs.semver)) {
-      modelSettingsVersion = SemanticVersion(QString(rhs.semver));
+    if (SemanticVersion().isValid(rhs.semver.toQString())) {
+      modelSettingsVersion = SemanticVersion(rhs.semver.toQString());
     }
     else {
-      qDebug() << "Invalid settings version:" << rhs.semver;
-      memset(rhs.semver, 0, sizeof(rhs.semver));
+      qDebug() << "Invalid settings version:" << rhs.semver.c_str();
+      rhs.semver.clear();
     }
   }
 
@@ -1355,7 +1355,7 @@ bool convert<ModelData>::decode(const Node& node, ModelData& rhs)
                << "Companion" << SemanticVersion(VERSION).toString();
     } else {
       QString prmpt = QCoreApplication::translate("YamlModelSettings", "Warning: '%1' has settings version %2 that is not supported by Companion %3!\n\nModel settings may be corrupted if you continue.");
-      prmpt = prmpt.arg(rhs.name).arg(modelSettingsVersion.toString()).arg(SemanticVersion(VERSION).toString());
+      prmpt = prmpt.arg(rhs.name.toQString()).arg(modelSettingsVersion.toString()).arg(SemanticVersion(VERSION).toString());
       QMessageBox msgBox;
       msgBox.setWindowTitle(QCoreApplication::translate("YamlModelSettings", "Read Model Settings"));
       msgBox.setText(prmpt);

companion/src/firmwares/edgetx/yaml_ops.cpp

@@ -72,18 +72,22 @@ void YamlValidateLabel(QString &input)
   delete lv;
 }
 
-void YamlValidateName(char *input, Board::Type board)
+QString YamlValidateName(const QString &input, Board::Type board)
 {
-  NameValidator *nv = new NameValidator(board);
+  NameValidator nv(board);
   QString in(input);
 
-  if (!nv->isValid(in)) {
-    nv->fixup(in);
+  if (!nv.isValid(in)) {
+    nv.fixup(in);
     in = in.trimmed();
   }
 
-  strcpy(input, in.toLatin1().data());
-  delete nv;
+  return in;
+}
+
+void YamlValidateName(char *input, Board::Type board)
+{
+  strcpy(input, YamlValidateName(QString(input), board).toLatin1().data());
 }
 
 namespace YAML {

companion/src/firmwares/edgetx/yaml_ops.h

@@ -25,6 +25,7 @@
 #include "helpers.h"
 #include "boards.h"
 #include "semanticversion.h"
+#include "boundedstring.h"
 
 #include <algorithm>
 #include <QString>
@@ -97,6 +98,17 @@ void operator >> (const YAML::Node& node, char (&value)[N])
   }
 }
 
+// Mirrors the char[] reader above: a scalar is assigned and truncated to the
+// field capacity. BoundedString<N> truncates to N bytes, matching the legacy
+// char[N+1] field (which keeps N usable bytes plus the NUL).
+template <size_t N>
+void operator >> (const YAML::Node& node, BoundedString<N>& value)
+{
+  if (node && node.IsScalar()) {
+    value = node.as<std::string>();
+  }
+}
+
 template <typename T, size_t N>
 void operator>>(const YAML::Node& node, T (&value)[N])
 {
@@ -127,10 +139,24 @@ void operator>>(const YAML::Node& node, T (&value)[N])
 }
 
 void YamlValidateName(char *input, Board::Type board);
+QString YamlValidateName(const QString &input, Board::Type board);
 void YamlValidateLabel(QString &input);
 
 namespace YAML {
 
+// Encode/decode for BoundedString<N> so `node["x"] = field` and `node.as<>()`
+// behave like the legacy char[] fields. Decode truncates to the capacity.
+template <size_t N>
+struct convert<BoundedString<N>> {
+  static Node encode(const BoundedString<N>& rhs) { return Node(rhs.str()); }
+  static bool decode(const Node& node, BoundedString<N>& rhs)
+  {
+    if (node && node.IsScalar())
+      rhs = node.as<std::string>();
+    return true;
+  }
+};
+
 std::string LookupValue(const YamlLookupTable& lut, const int& value);
 Node operator << (const YamlLookupTable& lut, const int& value);
 

companion/src/firmwares/modeldata.cpp

@@ -50,11 +50,11 @@ ModelData & ModelData::operator=(const ModelData & src)
 
 void ModelData::copy(const ModelData & src)
 {
-  memcpy(&semver, &src.semver, sizeof(semver));
+  semver = src.semver;
   used = src.used;
-  memcpy(&name, &src.name, sizeof(name));
-  memcpy(&filename, &src.filename, sizeof(filename));
-  memcpy(&labels, &src.labels, sizeof(labels));
+  name = src.name;
+  filename = src.filename;
+  labels = src.labels;
   modelIndex = src.modelIndex;
   modelUpdated = src.modelUpdated;
   modelErrors = src.modelErrors;
@@ -230,11 +230,11 @@ void ModelData::clear()
   // memset(reinterpret_cast<void *>(this), 0, sizeof(ModelData));
   // as struct contains complex data types eg std::string
 
-  memset(&semver, 0, sizeof(semver));
+  semver.clear();
   used = false;
-  memset(&name, 0, sizeof(name));
-  memset(&filename, 0, sizeof(filename));
-  memset(&labels, 0, sizeof(labels));
+  name.clear();
+  filename.clear();
+  labels.clear();
   modelIndex = -1;  // an invalid index, this is managed by the TreeView data model
   modelUpdated = false;
   modelErrors = false;
@@ -419,7 +419,7 @@ void ModelData::setDefaultValues(unsigned int id, const GeneralSettings & settin
 {
   clear();
   used = true;
-  sprintf(name, "MODEL%02d", id + 1);
+  name = QString::asprintf("MODEL%02d", id + 1);
 
   for (int i = 0; i < CPN_MAX_MODULES; i++) {
     moduleData[i].modelId = id + 1;
@@ -592,7 +592,7 @@ void ModelData::convert(RadioDataConversionState & cstate)
 {
   // Here we can add explicit conversions when moving from one board to another
 
-  QString origin = QString(name);
+  QString origin = name.toQString();
   if (origin.isEmpty())
     origin = QString::number(cstate.modelIdx+1);
   cstate.setOrigin(tr("Model: ") % origin);
@@ -2180,7 +2180,7 @@ const Board::SwitchType ModelData::getSwitchType(int sw, const GeneralSettings &
 
 QString ModelData::getChecklistFilename() const
 {
-  return QString(name).replace(" ", "_").append(".txt").toLower();
+  return name.toQString().replace(" ", "_").append(".txt").toLower();
 }
 
 void ModelData::gvarClear(const int index, bool updateRefs)

companion/src/firmwares/modeldata.h

@@ -21,6 +21,7 @@
 
 #pragma once
 
+#include "boundedstring.h"
 #include "constants.h"
 #include "curvedata.h"
 #include "customfunctiondata.h"
@@ -89,6 +90,9 @@ enum TrainerMode {
 };
 
 #define MODEL_NAME_LEN 15
+#define MODEL_FILENAME_LEN 16  // must match radio LEN_MODEL_FILENAME (dataconstants.h)
+#define MODEL_SEMVER_LEN 8
+#define MODEL_LABELS_LEN 99    // CSV of labels; was char labels[100]
 #define INPUT_NAME_LEN 4
 #define CPN_MAX_BITMAP_LEN 14
 
@@ -129,11 +133,11 @@ class ModelData {
     ModelData();
     ModelData(const ModelData & src);
 
-    char      semver[8 + 1];
+    BoundedString<MODEL_SEMVER_LEN> semver;
     bool      used;
-    char      name[MODEL_NAME_LEN + 1];
-    char      filename[16+1];
-    char      labels[100];
+    BoundedString<MODEL_NAME_LEN> name;
+    BoundedString<MODEL_FILENAME_LEN> filename;
+    BoundedString<MODEL_LABELS_LEN> labels;
     int       modelIndex;      // Companion only, temporary index position managed by data model.
     bool      modelUpdated;    // Companion only, used to highlight if changed in models list
     bool      modelErrors;     // Companion only, used to highlight if data errors in models list