Make single-argument string constructors explicit (#2707)

* Revise Linux platform types

* Fix regression in test autocoder

TimeBaseStoreType is not the same as TimeBase

* Revise test autocoder

Cast to the TimeBaseStore type with a known size

* Revise test autocoder

Remove unnecessary const casting

* Fix integer type mismatch in ComQueue

* Update fpp version

* Update fpp version

* Revise DpWriter unit tests

Remove manually written functions
Use new autocode in tester base

* Update fpp version

* Update fpp version

* Revise DpTest

Add check for priority set by dpGet

* Reivse StringType; add ExternalString

* Revise dp test

* Revise dp test; update fpp version

* Revise dp test

* Revise dp test

* Revise dp test

* Revise dp test

* Revise dp test

* Revise dp test

* Revise dp test

* Revise dp test

* Revise dp test

* Revise fpp version

* Update fpp version

* Revise ExternalString

* Reformat Fw/Serializable.{hpp,cpp}

* Reformat StringType.{hpp,cpp}

* Revise StringBase

* Update fpp version

* Remove trailing space

* Reformat code

* Revise dp test

* Reformat cpp and hpp files in Fw/Types

* Revise string types

* Revise string types

* Revise string types

* Remove EightyCharString

* Revise CmdString

* Revise LogString

* Revise PrmString

* Revise TextLogString

* Revise Test/String

* Revise TlmString

* Revise InternalInterfaceString

* Revise Os/QueueString

* Revise Os/TaskString

* Add missing type qualifier

* Fix warning in String.hpp

* Revise string types

* Remove stray character

* Revise StringBase

* Revise string types

* Revise String type

* Revise FileNameString

* Revise string types

* Add missing file

* Revise InternalInterfaceString

* Revise comment

* Revise TlmString

* Revise string types

* Revise log strings

* Revise PrmString

* Revise Os string classes

* Revise string types

* Revise string types

* Revise os strings

* Add test for ExternalString to TypesTest

* Revise ExternalString

* Revise ExternalString

* Update fpp version

* Format Fw/Types

* Add static serialized size to StringBase

* Update fpp version

* Update fpp version; revise string classes

* Revise event handling in DpWriter

* Revise string implementations

* Revise string implementations

* Refactor dp tests

* Add static assertion to PrmBuffer

For safety's sake!

* Revise string implementations; update fpp version

* Revise StringBase

Existing code was correct but somewhat terse and obscure
Refactor and add comments for clarity

* Fix tests to conform to FPP changes

* Remove helper scripts

* Add const qualifier

* Revise StringBase

* Revise StringBase

* Revise comment

* Update fpp version

* Fix code formatting

* Make constructors explicit

* Make string constructors explicit

* Make string constructors explicit

* Revise String.hpp

Remove explicit from char* constructor
The autocoder depends on the implicit constructor

* Fixing improper null-termination in fail case

* Fixing conversion warnings

* Removing broken link

---------

Co-authored-by: M Starch <LeStarch@googlemail.com>
Co-authored-by: Michael D Starch <Michael.D.Starch@jpl.nasa.gov>

FppTest/dp/test/ut/Tester.cpp

@@ -331,8 +331,7 @@ Fw::Time Tester::randomizeTestTime() {
 }
 
 void Tester::generateRandomString(Fw::StringBase& str) {
-    // Reserve enough space for max length plus null terminator
-    char buffer[MAX_STRING_LENGTH + 1];
+    char buffer[Fw::StringBase::BUFFER_SIZE(MAX_STRING_LENGTH)];
     // Pick a random string length
     const FwSizeType length = STest::Pick::lowerUpper(0, MAX_STRING_LENGTH);
     // Fill buffer with a random null-terminated string with that length

Fw/Cmd/CmdString.hpp

@@ -19,16 +19,16 @@ class CmdStringArg final : public StringBase {
     enum {
         SERIALIZED_TYPE_ID = FW_TYPEID_CMD_STR,
         STRING_SIZE = FW_CMD_STRING_MAX_SIZE,
-        SERIALIZED_SIZE = STRING_SIZE + sizeof(FwSizeStoreType)
+        SERIALIZED_SIZE = STATIC_SERIALIZED_SIZE(STRING_SIZE),
     };
 
     CmdStringArg() : StringBase() { *this = ""; }
 
-    CmdStringArg(const CmdStringArg& src) : StringBase() { *this = src; }
+    explicit CmdStringArg(const CmdStringArg& src) : StringBase() { *this = src; }
 
-    CmdStringArg(const StringBase& src) : StringBase() { *this = src; }
+    explicit CmdStringArg(const StringBase& src) : StringBase() { *this = src; }
 
-    CmdStringArg(const char* src) : StringBase() { *this = src; }
+    explicit CmdStringArg(const char* src) : StringBase() { *this = src; }
 
     ~CmdStringArg() {}
 
@@ -52,7 +52,7 @@ class CmdStringArg final : public StringBase {
     StringBase::SizeType getCapacity() const { return sizeof this->m_buf; }
 
   private:
-    char m_buf[CmdStringArg::STRING_SIZE];
+    char m_buf[BUFFER_SIZE(STRING_SIZE)];
 };
 }  // namespace Fw
 

Fw/Log/LogString.hpp

@@ -19,7 +19,7 @@ class LogStringArg final : public StringBase {
     enum {
         SERIALIZED_TYPE_ID = FW_TYPEID_LOG_STR,
         STRING_SIZE = FW_LOG_STRING_MAX_SIZE,
-        SERIALIZED_SIZE = STRING_SIZE + sizeof(FwSizeStoreType)
+        SERIALIZED_SIZE = STATIC_SERIALIZED_SIZE(STRING_SIZE)
     };
 
     LogStringArg() : StringBase() { *this = ""; }
@@ -52,7 +52,7 @@ class LogStringArg final : public StringBase {
     StringBase::SizeType getCapacity() const { return sizeof this->m_buf; }
 
   private:
-    char m_buf[LogStringArg::STRING_SIZE];
+    char m_buf[BUFFER_SIZE(STRING_SIZE)];
 };
 }  // namespace Fw
 

Fw/Log/TextLogString.hpp

@@ -19,7 +19,7 @@ class TextLogString final : public StringBase {
     enum {
         SERIALIZED_TYPE_ID = FW_TYPEID_LOG_STR,
         STRING_SIZE = FW_LOG_TEXT_BUFFER_SIZE,
-        SERIALIZED_SIZE = STRING_SIZE + sizeof(FwSizeStoreType)
+        SERIALIZED_SIZE = STATIC_SERIALIZED_SIZE(STRING_SIZE)
     };
 
     TextLogString() : StringBase() { *this = ""; }
@@ -52,7 +52,7 @@ class TextLogString final : public StringBase {
     StringBase::SizeType getCapacity() const { return sizeof this->m_buf; }
 
   private:
-    char m_buf[TextLogString::STRING_SIZE];
+    char m_buf[BUFFER_SIZE(STRING_SIZE)];
 };
 }  // namespace Fw
 

Fw/Logger/test/ut/LoggerMain.cpp

@@ -25,9 +25,9 @@ TEST(LoggerTests, RandomLoggerTests) {
     MockLogging::FakeLogger logger;
 
     // Create rules, and assign them into the array
-    LoggerRules::Register reg("Register");
-    LoggerRules::LogGood log("Log Successfully");
-    LoggerRules::LogBad nolog("Log unsuccessfully");
+    LoggerRules::Register reg(Fw::String("Register"));
+    LoggerRules::LogGood log(Fw::String("Log Successfully"));
+    LoggerRules::LogBad nolog(Fw::String("Log unsuccessfully"));
 
     // Setup a list of rules to choose from
     STest::Rule<MockLogging::FakeLogger>* rules[] = {
@@ -55,7 +55,7 @@ TEST(LoggerTests, BasicGoodLogger) {
     Fw::Logger::registerLogger(&logger);
     logger.s_current = &logger;
     // Basic logging
-    LoggerRules::LogGood log("Log Successfully");
+    LoggerRules::LogGood log(Fw::String("Log Successfully"));
     log.apply(logger);
 }
 /**
@@ -66,7 +66,7 @@ TEST(LoggerTests, BasicBadLogger) {
     MockLogging::FakeLogger logger;
     Fw::Logger::registerLogger(nullptr);
     logger.s_current = nullptr;
-    LoggerRules::LogBad log("Log Discarded");
+    LoggerRules::LogBad log(Fw::String("Log Discarded"));
     log.apply(logger);
 }
 
@@ -76,7 +76,7 @@ TEST(LoggerTests, BasicBadLogger) {
 TEST(LoggerTests, BasicRegLogger) {
     // Basic discard logging
     MockLogging::FakeLogger logger;
-    LoggerRules::Register reg("Register");
+    LoggerRules::Register reg(Fw::String("Register"));
     reg.apply(logger);
     reg.apply(logger);
     reg.apply(logger);

Fw/Prm/PrmBuffer.hpp

@@ -16,8 +16,14 @@
 #include <Fw/Types/Serializable.hpp>
 #include <Fw/Cfg/SerIds.hpp>
 
+#include "Fw/Types/StringBase.hpp"
+
+
 namespace Fw {
 
+    static_assert(FW_PARAM_BUFFER_MAX_SIZE >= StringBase::BUFFER_SIZE(FW_PARAM_STRING_MAX_SIZE),
+                  "param string must fit into param buffer");
+
     class ParamBuffer : public SerializeBufferBase {
         public:
 

Fw/Prm/PrmString.hpp

@@ -19,7 +19,7 @@ class ParamString : public StringBase {
     enum {
         SERIALIZED_TYPE_ID = FW_TYPEID_PRM_STR,
         STRING_SIZE = FW_PARAM_STRING_MAX_SIZE,
-        SERIALIZED_SIZE = STRING_SIZE + sizeof(FwSizeStoreType)
+        SERIALIZED_SIZE = STATIC_SERIALIZED_SIZE(STRING_SIZE)
     };
 
     ParamString() : StringBase() { *this = ""; }
@@ -52,7 +52,7 @@ class ParamString : public StringBase {
     StringBase::SizeType getCapacity() const { return sizeof this->m_buf; }
 
   private:
-    char m_buf[ParamString::STRING_SIZE];
+    char m_buf[BUFFER_SIZE(STRING_SIZE)];
 };
 }  // namespace Fw
 

Fw/Test/String.hpp

@@ -18,7 +18,7 @@ class String : public Fw::StringBase {
   public:
     enum {
         STRING_SIZE = 256,
-        SERIALIZED_SIZE = STRING_SIZE + sizeof(FwSizeStoreType)
+        SERIALIZED_SIZE = STATIC_SERIALIZED_SIZE(STRING_SIZE),
     };
 
     String() : StringBase() { *this = ""; }
@@ -49,7 +49,7 @@ class String : public Fw::StringBase {
     StringBase::SizeType getCapacity() const { return sizeof this->m_buf; }
 
   private:
-    char m_buf[String::STRING_SIZE];
+    char m_buf[BUFFER_SIZE(STRING_SIZE)];
 };
 }  // namespace Fw
 

Fw/Tlm/TlmString.hpp

@@ -19,7 +19,7 @@ class TlmString final : public StringBase {
     enum {
         SERIALIZED_TYPE_ID = FW_TYPEID_TLM_STR,
         STRING_SIZE = FW_TLM_STRING_MAX_SIZE,
-        SERIALIZED_SIZE = STRING_SIZE + sizeof(FwSizeStoreType)
+        SERIALIZED_SIZE = STATIC_SERIALIZED_SIZE(STRING_SIZE)
     };
 
     TlmString() : StringBase() { *this = ""; }
@@ -52,7 +52,7 @@ class TlmString final : public StringBase {
     StringBase::SizeType getCapacity() const { return sizeof this->m_buf; }
 
   private:
-    char m_buf[TlmString::STRING_SIZE];
+    char m_buf[BUFFER_SIZE(STRING_SIZE)];
 };
 }  // namespace Fw
 

Fw/Types/FileNameString.hpp

@@ -20,16 +20,16 @@ class FileNameString final : public StringBase {
     enum {
         SERIALIZED_TYPE_ID = FW_TYPEID_FILE_NAME_STRING,
         STRING_SIZE = FileNameStringSize,
-        SERIALIZED_SIZE = STRING_SIZE + sizeof(FwSizeStoreType)
+        SERIALIZED_SIZE = STATIC_SERIALIZED_SIZE(STRING_SIZE)
     };
 
     FileNameString() : StringBase() { *this = ""; }
 
-    FileNameString(const FileNameString& src) : StringBase() { *this = src; }
+    explicit FileNameString(const FileNameString& src) : StringBase() { *this = src; }
 
-    FileNameString(const StringBase& src) : StringBase() { *this = src; }
+    explicit FileNameString(const StringBase& src) : StringBase() { *this = src; }
 
-    FileNameString(const char* src) : StringBase() { *this = src; }
+    explicit FileNameString(const char* src) : StringBase() { *this = src; }
 
     ~FileNameString() {}
 
@@ -53,7 +53,7 @@ class FileNameString final : public StringBase {
     StringBase::SizeType getCapacity() const { return sizeof this->m_buf; }
 
   private:
-    char m_buf[FileNameString::STRING_SIZE];
+    char m_buf[BUFFER_SIZE(STRING_SIZE)];
 };
 }  // namespace Fw
 

Fw/Types/InternalInterfaceString.hpp

@@ -20,16 +20,16 @@ class InternalInterfaceString final : public StringBase {
     enum {
         SERIALIZED_TYPE_ID = FW_TYPEID_INTERNAL_INTERFACE_STRING,
         STRING_SIZE = FW_INTERNAL_INTERFACE_STRING_MAX_SIZE,
-        SERIALIZED_SIZE = STRING_SIZE + sizeof(FwSizeStoreType)
+        SERIALIZED_SIZE = STATIC_SERIALIZED_SIZE(STRING_SIZE)
     };
 
     InternalInterfaceString() : StringBase() { *this = ""; }
 
-    InternalInterfaceString(const InternalInterfaceString& src) : StringBase() { *this = src; }
+    explicit InternalInterfaceString(const InternalInterfaceString& src) : StringBase() { *this = src; }
 
-    InternalInterfaceString(const StringBase& src) : StringBase() { *this = src; }
+    explicit InternalInterfaceString(const StringBase& src) : StringBase() { *this = src; }
 
-    InternalInterfaceString(const char* src) : StringBase() { *this = src; }
+    explicit InternalInterfaceString(const char* src) : StringBase() { *this = src; }
 
     ~InternalInterfaceString() {}
 
@@ -53,7 +53,7 @@ class InternalInterfaceString final : public StringBase {
     StringBase::SizeType getCapacity() const { return sizeof this->m_buf; }
 
   private:
-    char m_buf[InternalInterfaceString::STRING_SIZE];
+    char m_buf[BUFFER_SIZE(STRING_SIZE)];
 };
 }  // namespace Fw
 

Fw/Types/ObjectName.hpp

@@ -19,16 +19,16 @@ class ObjectName final : public StringBase {
     enum {
         SERIALIZED_TYPE_ID = FW_TYPEID_OBJECT_NAME,
         STRING_SIZE = FW_OBJ_NAME_MAX_SIZE,
-        SERIALIZED_SIZE = STRING_SIZE + sizeof(FwSizeStoreType)
+        SERIALIZED_SIZE = STATIC_SERIALIZED_SIZE(STRING_SIZE)
     };
 
     ObjectName() : StringBase() { *this = ""; }
 
-    ObjectName(const ObjectName& src) : StringBase() { *this = src; }
+    explicit ObjectName(const ObjectName& src) : StringBase() { *this = src; }
 
-    ObjectName(const StringBase& src) : StringBase() { *this = src; }
+    explicit ObjectName(const StringBase& src) : StringBase() { *this = src; }
 
-    ObjectName(const char* src) : StringBase() { *this = src; }
+    explicit ObjectName(const char* src) : StringBase() { *this = src; }
 
     ~ObjectName() {}
 
@@ -52,7 +52,7 @@ class ObjectName final : public StringBase {
     StringBase::SizeType getCapacity() const { return sizeof this->m_buf; }
 
   private:
-    char m_buf[ObjectName::STRING_SIZE];
+    char m_buf[BUFFER_SIZE(STRING_SIZE)];
 };
 }  // namespace Fw
 

Fw/Types/String.hpp

@@ -19,14 +19,14 @@ class String final : public StringBase {
     enum {
         SERIALIZED_TYPE_ID = FW_TYPEID_FIXED_LENGTH_STRING,
         STRING_SIZE = FW_FIXED_LENGTH_STRING_SIZE,
-        SERIALIZED_SIZE = STRING_SIZE + sizeof(FwSizeStoreType)
+        SERIALIZED_SIZE = STATIC_SERIALIZED_SIZE(STRING_SIZE),
     };
 
     String() : StringBase() { *this = ""; }
 
-    String(const String& src) : StringBase() { *this = src; }
+    explicit String(const String& src) : StringBase() { *this = src; }
 
-    String(const StringBase& src) : StringBase() { *this = src; }
+    explicit String(const StringBase& src) : StringBase() { *this = src; }
 
     String(const char* src) : StringBase() { *this = src; }
 
@@ -52,7 +52,7 @@ class String final : public StringBase {
     StringBase::SizeType getCapacity() const { return sizeof this->m_buf; }
 
   private:
-    char m_buf[String::STRING_SIZE];
+    char m_buf[BUFFER_SIZE(STRING_SIZE)];
 };
 }  // namespace Fw
 

Fw/Types/StringBase.cpp

@@ -114,7 +114,16 @@ void StringBase::appendBuff(const CHAR* buff, SizeType size) {
 }
 
 StringBase::SizeType StringBase::length() const {
-    return static_cast<SizeType>(StringUtils::string_length(this->toChar(), this->getCapacity()));
+    const SizeType length = static_cast<SizeType>(StringUtils::string_length(this->toChar(), this->getCapacity()));
+    FW_ASSERT(length <= this->maxLength(), static_cast<FwAssertArgType>(length),
+              static_cast<FwAssertArgType>(this->maxLength()));
+    return length;
+}
+
+StringBase::SizeType StringBase::maxLength() const {
+    const SizeType capacity = this->getCapacity();
+    FW_ASSERT(capacity > 0, static_cast<FwAssertArgType>(capacity));
+    return capacity - 1;
 }
 
 StringBase::SizeType StringBase::serializedSize() const {
@@ -130,16 +139,34 @@ SerializeStatus StringBase::serialize(SerializeBufferBase& buffer) const {
 }
 
 SerializeStatus StringBase::serialize(SerializeBufferBase& buffer, SizeType maxLength) const {
-    SizeType len = FW_MIN(maxLength, this->length());
-    return buffer.serialize(reinterpret_cast<const U8*>(this->toChar()), len);
+    const FwSizeType len = FW_MIN(maxLength, this->length());
+    // Serialize length and then bytes
+    return buffer.serialize(reinterpret_cast<const U8*>(this->toChar()), len, Serialization::INCLUDE_LENGTH);
 }
 
 SerializeStatus StringBase::deserialize(SerializeBufferBase& buffer) {
-    SizeType maxSize = this->getCapacity() - 1;
+    // Get the max size of the deserialized string
+    const SizeType maxSize = this->maxLength();
+    // Initial estimate of actual size is max size
+    // This estimate is refined when calling the deserialize function below
+    SizeType actualSize = maxSize;
+    // Public interface returns const char*, but implementation needs char*
+    // So use const_cast
     CHAR* raw = const_cast<CHAR*>(this->toChar());
-    SerializeStatus stat = buffer.deserialize(reinterpret_cast<U8*>(raw), maxSize);
-    // Null terminate deserialized string
-    raw[maxSize] = 0;
+    // Deserialize length
+    // Fail if length exceeds max size (the initial value of actualSize)
+    // Otherwise deserialize length bytes and set actualSize to length
+    SerializeStatus stat = buffer.deserialize(reinterpret_cast<U8*>(raw), actualSize, Serialization::INCLUDE_LENGTH);
+    if (stat == FW_SERIALIZE_OK) {
+        // Deserialization succeeded: null-terminate string at actual size
+        FW_ASSERT(actualSize <= maxSize, static_cast<FwAssertArgType>(actualSize),
+                  static_cast<FwAssertArgType>(maxSize));
+        raw[actualSize] = 0;
+    } else {
+        // Deserialization failed: leave string unmodified, but ensure that it
+        // is null-terminated
+        raw[maxSize] = 0;
+    }
     return stat;
 }
 }  // namespace Fw