From bd2f09871d9950e18b2ac362a0afa03b5355e41f Mon Sep 17 00:00:00 2001 From: Michael D Starch Date: Wed, 29 Jan 2025 12:16:05 -0800 Subject: [PATCH] Removing printf from Svc. Partially Fixed #1708 --- .../LinuxSpiDriverComponentImpl.cpp | 9 +-- Fw/Types/StringBase.cpp | 30 +++++++-- Fw/Types/StringBase.hpp | 13 +++- Fw/Types/StringUtils.cpp | 7 -- Fw/Types/StringUtils.hpp | 21 ++---- Svc/ActiveTextLogger/ActiveTextLogger.cpp | 15 ++--- Svc/ActiveTextLogger/LogFile.cpp | 46 ++++--------- Svc/ActiveTextLogger/LogFile.hpp | 4 +- .../test/ut/ActiveTextLoggerTester.cpp | 20 +++--- .../AssertFatalAdapterComponentImpl.cpp | 4 +- Svc/ComLogger/ComLogger.cpp | 65 ++++++------------- Svc/ComLogger/ComLogger.hpp | 32 ++------- Svc/ComLogger/test/ut/ComLoggerTester.cpp | 13 ++-- Svc/FileManager/FileManager.cpp | 19 +++--- Svc/PassiveConsoleTextLogger/README | 6 -- Svc/TlmPacketizer/TlmPacketizer.cpp | 1 - Utils/CRCChecker.cpp | 20 +++--- 17 files changed, 128 insertions(+), 197 deletions(-) delete mode 100644 Svc/PassiveConsoleTextLogger/README diff --git a/Drv/LinuxSpiDriver/LinuxSpiDriverComponentImpl.cpp b/Drv/LinuxSpiDriver/LinuxSpiDriverComponentImpl.cpp index d437db8ad7..8e684f7277 100644 --- a/Drv/LinuxSpiDriver/LinuxSpiDriverComponentImpl.cpp +++ b/Drv/LinuxSpiDriver/LinuxSpiDriverComponentImpl.cpp @@ -13,7 +13,7 @@ #include #include #include -#include +#include #include #include #include @@ -76,10 +76,11 @@ namespace Drv { NATIVE_INT_TYPE ret; // Open: - char devName[256]; - Fw::StringUtils::format(devName, sizeof devName, "/dev/spidev%d.%d", device, select); + Fw::FileNameString devString; + Fw::FormatStatus formatStatus = devString.format("/dev/spidev%d.%d", device, select); + FW_ASSERT(formatStatus == Fw::FormatStatus::SUCCESS); - fd = ::open(devName, O_RDWR); + fd = ::open(devString.toChar(), O_RDWR); if (fd == -1) { this->log_WARNING_HI_SPI_OpenError(device,select,fd); return false; diff --git a/Fw/Types/StringBase.cpp b/Fw/Types/StringBase.cpp index 3a076e7a65..b86effa3e9 100644 --- a/Fw/Types/StringBase.cpp +++ b/Fw/Types/StringBase.cpp @@ -53,25 +53,45 @@ bool StringBase::operator==(const CHAR* other) const { return (result == 0); } -void StringBase::format(const CHAR* formatString, ...) { +FormatStatus StringBase::format(const CHAR* formatString, ...) { va_list args; va_start(args, formatString); - this->vformat(formatString, args); + FormatStatus status = this->vformat(formatString, args); va_end(args); + return status; } -void StringBase::vformat(const CHAR* formatString, va_list args) { +FormatStatus StringBase::vformat(const CHAR* formatString, va_list args) { CHAR* us = const_cast(this->toChar()); SizeType cap = this->getCapacity(); FW_ASSERT(us != nullptr); - FW_ASSERT(formatString != nullptr); + + // Check format string + if (formatString == nullptr) { + return FormatStatus::INVALID_FORMAT_STRING; + } + FwSizeType total_needed_size = 0; #if FW_USE_PRINTF_FAMILY_FUNCTIONS_IN_STRING_FORMATTING - (void) vsnprintf(us, cap, formatString, args); + // Check that the API size type fits in fprime size type + static_assert(std::numeric_limits::max() >= std::numeric_limits::max(), + "Range of PlatformIntType does not fit within range of FwSizeType"); + PlatformIntType total_needed_size_api = vsnprintf(us, cap, formatString, args); + // Check for error return, or a type overflow + if (total_needed_size_api < 0) { + return FormatStatus::OTHER_ERROR; + } + total_needed_size = static_cast(total_needed_size_api); #else + total_needed_size = StringUtils::string_length(format_string, cap); *this = formatString; #endif // Force null terminate us[cap - 1] = 0; + // Check for overflow + if (total_needed_size >= cap) { + return FormatStatus::OVERFLOWED; + } + return FormatStatus::SUCCESS; } bool StringBase::operator!=(const StringBase& other) const { diff --git a/Fw/Types/StringBase.hpp b/Fw/Types/StringBase.hpp index 5bd2b22b22..82df7def28 100644 --- a/Fw/Types/StringBase.hpp +++ b/Fw/Types/StringBase.hpp @@ -21,6 +21,15 @@ #endif namespace Fw { + +//! \brief status of format +enum class FormatStatus { + SUCCESS, //!< Format worked + OVERFLOWED, //!< Format overflowed + INVALID_FORMAT_STRING, //!< Format provided invalid format string + OTHER_ERROR //!< An error was returned from an underlying call +}; + class StringBase : public Serializable { public: using SizeType = NATIVE_UINT_TYPE; @@ -62,8 +71,8 @@ class StringBase : public Serializable { StringBase& operator=(const CHAR* src); //!< Assign CHAR* StringBase& operator=(const StringBase& src); //!< Assign another StringBase - void format(const CHAR* formatString, ...); //!< write formatted string to buffer - void vformat(const CHAR* formatString, va_list args); //!< write formatted string to buffer using va_list + FormatStatus format(const CHAR* formatString, ...); //!< write formatted string to buffer + FormatStatus vformat(const CHAR* formatString, va_list args); //!< write formatted string to buffer using va_list virtual SerializeStatus serialize(SerializeBufferBase& buffer) const; //!< serialization function virtual SerializeStatus serialize(SerializeBufferBase& buffer, SizeType maxLen) const; //!< serialization function diff --git a/Fw/Types/StringUtils.cpp b/Fw/Types/StringUtils.cpp index e8cb3428a0..df8147a685 100644 --- a/Fw/Types/StringUtils.cpp +++ b/Fw/Types/StringUtils.cpp @@ -73,10 +73,3 @@ FwSignedSizeType Fw::StringUtils::substring_find(const CHAR* source_string, // if we make it here, no matches were found return -1; } - -void Fw::StringUtils::format(CHAR* destination, Fw::StringBase::SizeType size, const CHAR* format, ...) { - va_list args; - va_start(args, format); - Fw::ExternalString(destination, size).vformat(format, args); - va_end(args); -} diff --git a/Fw/Types/StringUtils.hpp b/Fw/Types/StringUtils.hpp index ab05210e1a..a7acc53d80 100644 --- a/Fw/Types/StringUtils.hpp +++ b/Fw/Types/StringUtils.hpp @@ -1,7 +1,11 @@ +/** + * Fw/Types/StringUtils.hpp: + * + * C-string helper utilities. Note: wherever possible, use Fw::StringBase and derived classes instead of raw C-strings. + */ #ifndef FW_STRINGUTILS_HPP #define FW_STRINGUTILS_HPP #include -#include namespace Fw { namespace StringUtils { @@ -45,21 +49,6 @@ FwSizeType string_length(const CHAR* source, FwSizeType buffer_size); */ FwSignedSizeType substring_find(const CHAR* source_string, FwSizeType source_size, const CHAR* sub_string, FwSizeType sub_size); -//! \brief format a string and store into a destination buffer -//! -//! This function will format a string into a destination buffer. The destination buffer must be large enough to hold -//! the formatted string. The format delegates to the standard C library function vsnprintf underneath the hood. -//! -//! \warning if used in execution (i.e. a file path), assert FW_USE_PRINTF_FAMILY_FUNCTIONS_IN_STRING_FORMATTING. -//! ``` -//! static_assert(FW_USE_PRINTF_FAMILY_FUNCTIONS_IN_STRING_FORMATTING, "String formatting disabled") -//! ``` -//! \param destination: destination buffer to hold formatted string -//! \param size: size of the destination buffer. Output will be truncated to this length -//! \param format: constant format string -//! \param ...: variable arguments to to pass to the format string -void format(CHAR* destination, StringBase::SizeType size, const CHAR* format, ...); - enum StringToNumberStatus { SUCCESSFUL_CONVERSION, //!< Output should be valid NULL_INPUT, //!< A null string was supplied diff --git a/Svc/ActiveTextLogger/ActiveTextLogger.cpp b/Svc/ActiveTextLogger/ActiveTextLogger.cpp index 4eb5f1c571..04e835479f 100644 --- a/Svc/ActiveTextLogger/ActiveTextLogger.cpp +++ b/Svc/ActiveTextLogger/ActiveTextLogger.cpp @@ -72,19 +72,14 @@ namespace Svc { severityString = "SEVERITY ERROR"; break; } - - // TODO: Add calling task id to format string - char textStr[FW_INTERNAL_INTERFACE_STRING_MAX_SIZE]; - - (void) snprintf(textStr, - FW_INTERNAL_INTERFACE_STRING_MAX_SIZE, - "EVENT: (%" PRI_FwEventIdType ") (%" PRI_FwTimeBaseStoreType ":%" PRIu32 ",%" PRIu32 ") %s: %s\n", - id, static_cast(timeTag.getTimeBase()), timeTag.getSeconds(), timeTag.getUSeconds(), - severityString, text.toChar()); + // Overflow is allowed and truncation accepted + Fw::InternalInterfaceString intText; + (void) intText.format("EVENT: (%" PRI_FwEventIdType ") (%" PRI_FwTimeBaseStoreType ":%" PRIu32 ",%" PRIu32 ") %s: %s\n", + id, static_cast(timeTag.getTimeBase()), timeTag.getSeconds(), timeTag.getUSeconds(), + severityString, text.toChar()); // Call internal interface so that everything else is done on component thread, // this helps ensure consistent ordering of the printed text: - Fw::InternalInterfaceString intText(textStr); this->TextQueue_internalInterfaceInvoke(intText); } diff --git a/Svc/ActiveTextLogger/LogFile.cpp b/Svc/ActiveTextLogger/LogFile.cpp index 0d7154cca7..4045f3ea03 100644 --- a/Svc/ActiveTextLogger/LogFile.cpp +++ b/Svc/ActiveTextLogger/LogFile.cpp @@ -87,60 +87,40 @@ namespace Svc { this->m_openFile = false; this->m_file.close(); } + Fw::FileNameString searchFilename; + Fw::FormatStatus formatStatus = searchFilename.format("%s", fileName); // If file name is too large, return failure: - FwSizeType fileNameSize = Fw::StringUtils::string_length(fileName, static_cast(Fw::String::STRING_SIZE)); - if (fileNameSize == Fw::String::STRING_SIZE) { + if (formatStatus != Fw::FormatStatus::SUCCESS) { return false; } - U32 suffix = 0; - FwSignedSizeType tmp; - char fileNameFinal[Fw::String::STRING_SIZE]; - (void) strncpy(fileNameFinal,fileName, - Fw::String::STRING_SIZE); - fileNameFinal[Fw::String::STRING_SIZE-1] = 0; - // Check if file already exists, and if it does try to tack on a suffix. - // Quit after 10 suffix addition tries (first try is w/ the original name). + // Quit after maxBackups suffix addition tries (first try is w/ the original name). + U32 suffix = 0; bool failedSuffix = false; - while (Os::FileSystem::getFileSize(fileNameFinal,tmp) == Os::FileSystem::OP_OK) { - - // If the file name was the max size, then can't append a suffix, - // so just fail: - if (fileNameSize == (Fw::String::STRING_SIZE-1)) { - return false; - } - + FwSignedSizeType fileSize = 0; + while (Os::FileSystem::getFileSize(searchFilename.toChar(), fileSize) == Os::FileSystem::OP_OK) { // Not able to create a new non-existing file in maxBackups tries, then mark that it failed: if (suffix >= maxBackups) { failedSuffix = true; break; } - - NATIVE_INT_TYPE stat = snprintf(fileNameFinal,Fw::String::STRING_SIZE, - "%s%" PRIu32,fileName,suffix); - - // If there was error, then just fail: - if (stat <= 0) { + // Format and check for error and overflows + formatStatus = searchFilename.format("%s%" PRIu32, fileName, suffix); + if (formatStatus != Fw::FormatStatus::SUCCESS) { return false; } - - // There should never be truncation: - FW_ASSERT(stat < Fw::String::STRING_SIZE); - ++suffix; } // If failed trying to make a new file, just use the original file if (failedSuffix) { - (void) strncpy(fileNameFinal,fileName, - Fw::String::STRING_SIZE); - fileNameFinal[Fw::String::STRING_SIZE-1] = 0; + searchFilename = fileName; } // Open the file (using CREATE so that it truncates an already existing file): - Os::File::Status stat = this->m_file.open(fileNameFinal, Os::File::OPEN_CREATE, Os::File::OverwriteType::OVERWRITE); + Os::File::Status stat = this->m_file.open(searchFilename.toChar(), Os::File::OPEN_CREATE, Os::File::OverwriteType::OVERWRITE); // Bad status when trying to open the file: if (stat != Os::File::OP_OK) { @@ -149,7 +129,7 @@ namespace Svc { this->m_currentFileSize = 0; this->m_maxFileSize = maxSize; - this->m_fileName = fileNameFinal; + this->m_fileName = searchFilename; this->m_openFile = true; return true; diff --git a/Svc/ActiveTextLogger/LogFile.hpp b/Svc/ActiveTextLogger/LogFile.hpp index ae77f8d792..1aa431bf67 100644 --- a/Svc/ActiveTextLogger/LogFile.hpp +++ b/Svc/ActiveTextLogger/LogFile.hpp @@ -6,7 +6,7 @@ #ifndef SVCLOGFILE_HPP_ #define SVCLOGFILE_HPP_ -#include +#include #include #include @@ -55,7 +55,7 @@ namespace Svc { // ---------------------------------------------------------------------- // The name of the file to text logs to: - Fw::String m_fileName; + Fw::FileNameString m_fileName; // The file to write text logs to: Os::File m_file; diff --git a/Svc/ActiveTextLogger/test/ut/ActiveTextLoggerTester.cpp b/Svc/ActiveTextLogger/test/ut/ActiveTextLoggerTester.cpp index 25c951e1ff..d484698326 100644 --- a/Svc/ActiveTextLogger/test/ut/ActiveTextLoggerTester.cpp +++ b/Svc/ActiveTextLogger/test/ut/ActiveTextLoggerTester.cpp @@ -253,13 +253,13 @@ namespace Svc { printf("Testing file name larger than string size\n"); - // Setup filename larger than 80 char: - char longFileName[Fw::String::STRING_SIZE + 1]; - for (U32 i = 0; i < Fw::String::STRING_SIZE; ++i) { + // Setup filename larger than file name string can accept + // Maximum valid file name is Fw::FileNameString::STRING_SIZE, add 1 to be too long, and an extra for the \0 + char longFileName[Fw::FileNameString::STRING_SIZE + 2]; + for (U32 i = 0; i < Fw::FileNameString::STRING_SIZE + 1; ++i) { longFileName[i] = 'a'; } - longFileName[Fw::String::STRING_SIZE] = 0; - + longFileName[Fw::FileNameString::STRING_SIZE + 1] = 0; stat = this->component.set_log_file(longFileName,50); // Verify file not made: @@ -269,11 +269,12 @@ namespace Svc { Os::FileSystem::getFileSize(longFileName,tmp)); printf("Testing file name of max size and file already exists\n"); - char longFileNameDup[Fw::String::STRING_SIZE]; - for (U32 i = 0; i < Fw::String::STRING_SIZE; ++i) { + // Maximum valid file name is Fw::FileNameString::STRING_SIZE add one for \0 + char longFileNameDup[Fw::FileNameString::STRING_SIZE + 1]; + for (U32 i = 0; i < Fw::FileNameString::STRING_SIZE; ++i) { longFileNameDup[i] = 'a'; } - longFileNameDup[Fw::String::STRING_SIZE-1] = 0; + longFileNameDup[Fw::FileNameString::STRING_SIZE] = 0; stat = this->component.set_log_file(longFileNameDup,50); @@ -311,9 +312,6 @@ namespace Svc { char baseNameWithSuffix[128]; U32 i; for (i = 0; i < 10; ++i) { - - //printf("<< %i\n",i); - stat = this->component.set_log_file(baseName,50); snprintf(baseNameWithSuffix, sizeof(baseNameWithSuffix), "%s%d",baseName,i); diff --git a/Svc/AssertFatalAdapter/AssertFatalAdapterComponentImpl.cpp b/Svc/AssertFatalAdapter/AssertFatalAdapterComponentImpl.cpp index 88e4a40f68..6618d9c2f9 100644 --- a/Svc/AssertFatalAdapter/AssertFatalAdapterComponentImpl.cpp +++ b/Svc/AssertFatalAdapter/AssertFatalAdapterComponentImpl.cpp @@ -118,9 +118,7 @@ namespace Svc { CHAR msg[Fw::StringBase::BUFFER_SIZE(FW_ASSERT_TEXT_SIZE)] = {0}; Fw::defaultReportAssert(file,lineNo,numArgs,arg1,arg2,arg3,arg4,arg5,arg6,msg,sizeof(msg)); - // fprintf(stderr... allocates large buffers on stack as stderr is unbuffered by the OS - // and this can conflict with the traditionally smaller stack sizes. - printf("%s\n", msg); + Fw::Logger::log("%s\n", msg); // Handle the case where the ports aren't connected yet if (not this->isConnected_Log_OutputPort(0)) { diff --git a/Svc/ComLogger/ComLogger.cpp b/Svc/ComLogger/ComLogger.cpp index 063318d0de..7a87a18b03 100644 --- a/Svc/ComLogger/ComLogger.cpp +++ b/Svc/ComLogger/ComLogger.cpp @@ -56,13 +56,9 @@ namespace Svc { if( this->m_storeBufferLength ) { FW_ASSERT(maxFileSize > sizeof(U16), static_cast(maxFileSize)); } - - FW_ASSERT(Fw::StringUtils::string_length(incomingFilePrefix, static_cast(sizeof(this->m_filePrefix))) < sizeof(this->m_filePrefix), - static_cast(Fw::StringUtils::string_length(incomingFilePrefix, static_cast(sizeof(this->m_filePrefix)))), - static_cast(sizeof(this->m_filePrefix))); // ensure that file prefix is not too big - - (void)Fw::StringUtils::string_copy(this->m_filePrefix, incomingFilePrefix, static_cast(sizeof(this->m_filePrefix))); - + // Assign the prefix checking if it is too big + Fw::FormatStatus formatStatus = this->m_filePrefix.format("%s", incomingFilePrefix); + FW_ASSERT(formatStatus == Fw::FormatStatus::SUCCESS); this->m_initialized = true; } @@ -165,42 +161,23 @@ namespace Svc { return; } - U32 bytesCopied; - // Create filename: Fw::Time timestamp = getTime(); - memset(this->m_fileName, 0, sizeof(this->m_fileName)); - bytesCopied = static_cast(snprintf( - this->m_fileName, - sizeof(this->m_fileName), - "%s_%" PRI_FwTimeBaseStoreType "_%" PRIu32 "_%06" PRIu32 ".com", - this->m_filePrefix, - static_cast(timestamp.getTimeBase()), - timestamp.getSeconds(), - timestamp.getUSeconds())); - - // "A return value of size or more means that the output was truncated" - // See here: http://linux.die.net/man/3/snprintf - FW_ASSERT( bytesCopied < sizeof(this->m_fileName) ); - - // Create sha filename: - bytesCopied = static_cast(snprintf( - this->m_hashFileName, - sizeof(this->m_hashFileName), - "%s_%" PRI_FwTimeBaseStoreType "_%" PRIu32 "_%06" PRIu32 ".com%s", - this->m_filePrefix, - static_cast(timestamp.getTimeBase()), - timestamp.getSeconds(), - timestamp.getUSeconds(), - Utils::Hash::getFileExtensionString())); - FW_ASSERT( bytesCopied < sizeof(this->m_hashFileName) ); - - Os::File::Status ret = m_file.open(this->m_fileName, Os::File::OPEN_WRITE); + Fw::FormatStatus formatStatus = this->m_fileName.format( + "%s_%" PRI_FwTimeBaseStoreType "_%" PRIu32 "_%06" PRIu32 ".com", + this->m_filePrefix.toChar(), + static_cast(timestamp.getTimeBase()), + timestamp.getSeconds(), + timestamp.getUSeconds()); + FW_ASSERT(formatStatus == Fw::FormatStatus::SUCCESS); + this->m_hashFileName.format("%s%s", this->m_fileName.toChar(), Utils::Hash::getFileExtensionString()); + FW_ASSERT(formatStatus == Fw::FormatStatus::SUCCESS); + + Os::File::Status ret = m_file.open(this->m_fileName.toChar(), Os::File::OPEN_WRITE); if( Os::File::OP_OK != ret ) { if( !this->m_openErrorOccurred ) { // throttle this event, otherwise a positive // feedback event loop can occur! - Fw::LogStringArg logStringArg(this->m_fileName); - this->log_WARNING_HI_FileOpenError(ret, logStringArg); + this->log_WARNING_HI_FileOpenError(ret, this->m_fileName); } this->m_openErrorOccurred = true; } else { @@ -230,8 +207,7 @@ namespace Svc { this->m_fileMode = CLOSED; // Send event: - Fw::LogStringArg logStringArg(this->m_fileName); - this->log_DIAGNOSTIC_FileClosed(logStringArg); + this->log_DIAGNOSTIC_FileClosed(this->m_fileName); } } @@ -271,8 +247,7 @@ namespace Svc { if( Os::File::OP_OK != ret || size != static_cast(length) ) { if( !this->m_writeErrorOccurred ) { // throttle this event, otherwise a positive // feedback event loop can occur! - Fw::LogStringArg logStringArg(this->m_fileName); - this->log_WARNING_HI_FileWriteError(ret, static_cast(size), length, logStringArg); + this->log_WARNING_HI_FileWriteError(ret, static_cast(size), length, this->m_fileName); } this->m_writeErrorOccurred = true; return false; @@ -287,11 +262,9 @@ namespace Svc { ) { Os::ValidateFile::Status validateStatus; - validateStatus = Os::ValidateFile::createValidation(this->m_fileName, this->m_hashFileName); + validateStatus = Os::ValidateFile::createValidation(this->m_fileName.toChar(), this->m_hashFileName.toChar()); if( Os::ValidateFile::VALIDATION_OK != validateStatus ) { - Fw::LogStringArg logStringArg1(this->m_fileName); - Fw::LogStringArg logStringArg2(this->m_hashFileName); - this->log_WARNING_LO_FileValidationError(logStringArg1, logStringArg2, validateStatus); + this->log_WARNING_LO_FileValidationError(this->m_fileName, this->m_hashFileName, validateStatus); } } } diff --git a/Svc/ComLogger/ComLogger.hpp b/Svc/ComLogger/ComLogger.hpp index 2122ed254f..67028bfc98 100644 --- a/Svc/ComLogger/ComLogger.hpp +++ b/Svc/ComLogger/ComLogger.hpp @@ -12,25 +12,11 @@ #include #include #include - +#include #include #include #include -// some limits.h don't have PATH_MAX -#ifdef PATH_MAX -#define COMLOGGER_PATH_MAX PATH_MAX -#else -#define COMLOGGER_PATH_MAX 255 -#endif - -// some limits.h don't have NAME_MAX -#ifdef NAME_MAX -#define COMLOGGER_NAME_MAX NAME_MAX -#else -#define COMLOGGER_NAME_MAX 255 -#endif - namespace Svc { class ComLogger : @@ -90,17 +76,8 @@ namespace Svc { U32 key /*!< Value to return to pinger*/ ); - // ---------------------------------------------------------------------- - // Constants: - // ---------------------------------------------------------------------- - // The maximum size of a filename - enum { - MAX_FILENAME_SIZE = COMLOGGER_NAME_MAX, - MAX_PATH_SIZE = COMLOGGER_PATH_MAX - }; - // The filename data: - CHAR m_filePrefix[MAX_FILENAME_SIZE + MAX_PATH_SIZE]; + Fw::FileNameString m_filePrefix; U32 m_maxFileSize; // ---------------------------------------------------------------------- @@ -113,8 +90,9 @@ namespace Svc { FileMode m_fileMode; Os::File m_file; - CHAR m_fileName[MAX_FILENAME_SIZE + MAX_PATH_SIZE]; - CHAR m_hashFileName[MAX_FILENAME_SIZE + MAX_PATH_SIZE]; + + Fw::FileNameString m_fileName; + Fw::FileNameString m_hashFileName; U32 m_byteCount; bool m_writeErrorOccurred; bool m_openErrorOccurred; diff --git a/Svc/ComLogger/test/ut/ComLoggerTester.cpp b/Svc/ComLogger/test/ut/ComLoggerTester.cpp index 803819ab4b..0ab52efbb2 100644 --- a/Svc/ComLogger/test/ut/ComLoggerTester.cpp +++ b/Svc/ComLogger/test/ut/ComLoggerTester.cpp @@ -138,7 +138,7 @@ namespace Svc { // A new file should have been opened from the previous loop iteration: if( j > 0 ) { ASSERT_TRUE(comLogger.m_fileMode == ComLogger::OPEN); - ASSERT_TRUE(strcmp(static_cast(comLogger.m_fileName), fileName) == 0 ); + ASSERT_TRUE(strcmp(comLogger.m_fileName.toChar(), fileName) == 0 ); } // Make sure we got a closed file event: @@ -261,7 +261,7 @@ namespace Svc { // A new file should have been opened from the previous loop iteration: if( j > 0 ) { ASSERT_TRUE(comLogger.m_fileMode == ComLogger::OPEN); - ASSERT_TRUE(strcmp(static_cast(comLogger.m_fileName), fileName) == 0 ); + ASSERT_TRUE(strcmp(comLogger.m_fileName.toChar(), fileName) == 0 ); } // Make sure we got a closed file event: @@ -323,7 +323,7 @@ namespace Svc { memset(filePrefix, 0, sizeof(filePrefix)); snprintf(filePrefix, sizeof(filePrefix), "illegal/fname?;\\*"); - strncpy(static_cast(comLogger.m_filePrefix), filePrefix, sizeof(comLogger.m_filePrefix)); + comLogger.m_filePrefix = filePrefix; ASSERT_TRUE(comLogger.m_fileMode == ComLogger::CLOSED); ASSERT_EVENTS_SIZE(0); @@ -358,7 +358,7 @@ namespace Svc { memset(filePrefix, 0, sizeof(filePrefix)); snprintf(filePrefix, sizeof(filePrefix), "good_"); - strncpy(comLogger.m_filePrefix, filePrefix, sizeof(comLogger.m_filePrefix)); + comLogger.m_filePrefix = filePrefix; ASSERT_TRUE(comLogger.m_fileMode == ComLogger::CLOSED); @@ -381,8 +381,7 @@ namespace Svc { memset(fileName, 0, sizeof(fileName)); memset(filePrefix, 0, sizeof(filePrefix)); snprintf(filePrefix, sizeof(filePrefix), "illegal/fname?;\\*"); - - strncpy(static_cast(comLogger.m_filePrefix), filePrefix, sizeof(comLogger.m_filePrefix)); + comLogger.m_filePrefix = filePrefix; ASSERT_TRUE(comLogger.m_fileMode == ComLogger::CLOSED); @@ -628,7 +627,7 @@ namespace Svc { // A new file should have been opened from the previous loop iteration: if( j > 0 ) { ASSERT_TRUE(comLogger.m_fileMode == ComLogger::OPEN); - ASSERT_TRUE(strcmp(static_cast(comLogger.m_fileName), fileName) == 0 ); + ASSERT_TRUE(strcmp(comLogger.m_fileName.toChar(), fileName) == 0 ); } // Make sure we got a closed file event: diff --git a/Svc/FileManager/FileManager.cpp b/Svc/FileManager/FileManager.cpp index ebca6f3702..547b4e4d97 100644 --- a/Svc/FileManager/FileManager.cpp +++ b/Svc/FileManager/FileManager.cpp @@ -13,6 +13,7 @@ #include #include +#include "Fw/Types/ExternalString.hpp" #include "Svc/FileManager/FileManager.hpp" #include "Fw/Types/Assert.hpp" #include @@ -261,18 +262,20 @@ namespace Svc { const Fw::CmdStringArg& logFileName ) const { + // Create a buffer of at least enough size for storing the eval string less the 2 %s tokens, two command strings, + // and a null terminator at the end const char evalStr[] = "eval '%s' 1>>%s 2>&1\n"; - const U32 bufferSize = sizeof(evalStr) - 4 + 2 * FW_CMD_STRING_MAX_SIZE; + constexpr U32 bufferSize = (sizeof(evalStr) - 4) + (2 * FW_CMD_STRING_MAX_SIZE) + 1; char buffer[bufferSize]; - NATIVE_INT_TYPE bytesCopied = snprintf( - buffer, sizeof(buffer), evalStr, - command.toChar(), - logFileName.toChar() - ); - FW_ASSERT(static_cast(bytesCopied) < sizeof(buffer)); + // Wrap that buffer in an external string for formatting purposes + Fw::ExternalString stringBuffer(buffer, bufferSize); + Fw::FormatStatus formatStatus = stringBuffer.format(evalStr, command.toChar(), logFileName.toChar()); + // Since the buffer is exactly sized, the only error can occur is a software error not caused by ground + FW_ASSERT(formatStatus == Fw::FormatStatus::SUCCESS); - const int status = system(buffer); + // Call the system + const int status = system(stringBuffer.toChar()); return status; } diff --git a/Svc/PassiveConsoleTextLogger/README b/Svc/PassiveConsoleTextLogger/README deleted file mode 100644 index 3d9d8f1c3d..0000000000 --- a/Svc/PassiveConsoleTextLogger/README +++ /dev/null @@ -1,6 +0,0 @@ -This component implements the text log interface. It is a simple version that simply uses printf to dump the text to standard out. -It has some variants depending on the processor target. - -PassiveTextLoggerComponentAi.xml - XML that defines component -ConsoleTextLoggerImpl.hpp - Header for implementation class -ConsoleTextLoggerImplCommon.cpp - Contains common class methods such as constructor and destructor. Sends message to Fw::Logger. diff --git a/Svc/TlmPacketizer/TlmPacketizer.cpp b/Svc/TlmPacketizer/TlmPacketizer.cpp index 34cdcaa1eb..e518c01a0b 100644 --- a/Svc/TlmPacketizer/TlmPacketizer.cpp +++ b/Svc/TlmPacketizer/TlmPacketizer.cpp @@ -221,7 +221,6 @@ void TlmPacketizer ::TlmRecv_handler(const NATIVE_INT_TYPE portNum, // check if current packet has this channel if (entryToUse->packetOffset[pkt] != -1) { // get destination address - // printf("PK %d CH: %d\n",this->m_fillBuffers[pkt].id,id); this->m_lock.lock(); this->m_fillBuffers[pkt].updated = true; this->m_fillBuffers[pkt].latestTime = timeTag; diff --git a/Utils/CRCChecker.cpp b/Utils/CRCChecker.cpp index 705477b4df..19492656d0 100644 --- a/Utils/CRCChecker.cpp +++ b/Utils/CRCChecker.cpp @@ -15,9 +15,11 @@ #include #include #include -#include +#include namespace Utils { +static_assert(FW_USE_PRINTF_FAMILY_FUNCTIONS_IN_STRING_FORMATTING, + "Cannot use CRC checker without full string formatting"); crc_stat_t create_checksum_file(const char* const fname) { @@ -35,7 +37,7 @@ namespace Utils { FwSignedSizeType int_file_size; FwSignedSizeType bytes_to_read; FwSignedSizeType bytes_to_write; - CHAR hashFilename[CRC_MAX_FILENAME_SIZE]; + Fw::FileNameString hashFilename; U8 block_data[CRC_FILE_READ_BLOCK]; fs_stat = Os::FileSystem::getFileSize(fname, filesize); @@ -89,10 +91,10 @@ namespace Utils { hash.final(checksum); // open checksum file - FW_ASSERT(CRC_MAX_FILENAME_SIZE > (Fw::StringUtils::string_length(fname, CRC_MAX_FILENAME_SIZE) + sizeof HASH_EXTENSION_STRING)); - Fw::StringUtils::format(hashFilename, CRC_MAX_FILENAME_SIZE, "%s%s", fname, HASH_EXTENSION_STRING); + Fw::FormatStatus formatStatus = hashFilename.format("%s%s", fname, HASH_EXTENSION_STRING); + FW_ASSERT(formatStatus == Fw::FormatStatus::SUCCESS); - stat = f.open(hashFilename, Os::File::OPEN_WRITE); + stat = f.open(hashFilename.toChar(), Os::File::OPEN_WRITE); if(stat != Os::File::OP_OK) { return FAILED_FILE_CRC_OPEN; @@ -116,13 +118,13 @@ namespace Utils { crc_stat_t read_crc32_from_file(const char* const fname, U32 &checksum_from_file) { Os::File f; Os::File::Status stat; - char hashFilename[CRC_MAX_FILENAME_SIZE]; + Fw::FileNameString hashFilename; FW_ASSERT(fname != nullptr); // open checksum file - FW_ASSERT(CRC_MAX_FILENAME_SIZE > (Fw::StringUtils::string_length(fname, CRC_MAX_FILENAME_SIZE) + sizeof HASH_EXTENSION_STRING)); - Fw::StringUtils::format(hashFilename, CRC_MAX_FILENAME_SIZE, "%s%s", fname, HASH_EXTENSION_STRING); + Fw::FormatStatus formatStatus = hashFilename.format("%s%s", fname, HASH_EXTENSION_STRING); + FW_ASSERT(formatStatus == Fw::FormatStatus::SUCCESS); - stat = f.open(hashFilename, Os::File::OPEN_READ); + stat = f.open(hashFilename.toChar(), Os::File::OPEN_READ); if(stat != Os::File::OP_OK) { return FAILED_FILE_CRC_OPEN;