Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Removing printf from Svc. Partially Fixed #1708 #3170

Merged
merged 1 commit into from
Jan 29, 2025
Merged

Removing printf from Svc. Partially Fixed #1708 #3170

merged 1 commit into from
Jan 29, 2025

Conversation

LeStarch
Copy link
Collaborator

Related Issue(s)
Has Unit Tests (y/n)
Documentation Included (y/n)

Change Description

Removes printf from service. Switches to Fw::FileNameString for filenames to avoid the stack buffer allocation and switch to fprime string types.

@LeStarch LeStarch requested a review from thomas-bc January 29, 2025 20:17
github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 29, 2025
View reviewed changes
Fw/Types/StringBase.cpp
// Check that the API size type fits in fprime size type
static_assert(std::numeric_limits<FwSizeType>::max() >= std::numeric_limits<PlatformIntType>::max(),
"Range of PlatformIntType does not fit within range of FwSizeType");
PlatformIntType total_needed_size_api = vsnprintf(us, cap, formatString, args);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter args has not been checked.
Svc/ActiveTextLogger/ActiveTextLogger.cpp
// 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<FwTimeBaseStoreType>(timeTag.getTimeBase()), timeTag.getSeconds(), timeTag.getUSeconds(),

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter id has not been checked.
Svc/ActiveTextLogger/ActiveTextLogger.cpp
// 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<FwTimeBaseStoreType>(timeTag.getTimeBase()), timeTag.getSeconds(), timeTag.getUSeconds(),

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter timeTag has not been checked.
Svc/ActiveTextLogger/ActiveTextLogger.cpp
Fw::InternalInterfaceString intText;
(void) intText.format("EVENT: (%" PRI_FwEventIdType ") (%" PRI_FwTimeBaseStoreType ":%" PRIu32 ",%" PRIu32 ") %s: %s\n",
id, static_cast<FwTimeBaseStoreType>(timeTag.getTimeBase()), timeTag.getSeconds(), timeTag.getUSeconds(),
severityString, text.toChar());

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter text has not been checked.
Svc/ActiveTextLogger/LogFile.cpp
(void) strncpy(fileNameFinal,fileName,
Fw::String::STRING_SIZE);
fileNameFinal[Fw::String::STRING_SIZE-1] = 0;
searchFilename = fileName;

Check warning

Code scanning / CodeQL

Unchecked return value Warning

The return value of non-void function
operator=
Loading
is not checked.
Svc/ActiveTextLogger/LogFile.cpp
@@ -149,7 +129,7 @@

this->m_currentFileSize = 0;
this->m_maxFileSize = maxSize;
this->m_fileName = fileNameFinal;
this->m_fileName = searchFilename;

Check warning

Code scanning / CodeQL

Unchecked return value Warning

The return value of non-void function
operator=
Loading
is not checked.
Svc/ComLogger/ComLogger.cpp
timestamp.getSeconds(),
timestamp.getUSeconds());
FW_ASSERT(formatStatus == Fw::FormatStatus::SUCCESS);
this->m_hashFileName.format("%s%s", this->m_fileName.toChar(), Utils::Hash::getFileExtensionString());

Check warning

Code scanning / CodeQL

Unchecked return value Warning

The return value of non-void function
format
Loading
is not checked.
Svc/FileManager/FileManager.cpp
FW_ASSERT(static_cast<NATIVE_UINT_TYPE>(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());

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter command has not been checked.
Svc/FileManager/FileManager.cpp
FW_ASSERT(static_cast<NATIVE_UINT_TYPE>(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());

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter logFileName has not been checked.
github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 29, 2025
View reviewed changes
Fw/Types/StringBase.cpp
@@ -53,25 +53,45 @@
return (result == 0);
}

void StringBase::format(const CHAR* formatString, ...) {
FormatStatus StringBase::format(const CHAR* formatString, ...) {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

formatString uses the basic integral type char rather than a typedef with size and signedness.
Fw/Types/StringBase.cpp
}

void StringBase::vformat(const CHAR* formatString, va_list args) {
FormatStatus StringBase::vformat(const CHAR* formatString, va_list args) {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

formatString uses the basic integral type char rather than a typedef with size and signedness.
Fw/Types/StringBase.cpp
if (formatString == nullptr) {
return FormatStatus::INVALID_FORMAT_STRING;
}
FwSizeType total_needed_size = 0;

Check notice

Code scanning / CodeQL

Use of basic integral type Note

total_needed_size uses the basic integral type unsigned long rather than a typedef with size and signedness.
Fw/Types/StringBase.cpp
// Check that the API size type fits in fprime size type
static_assert(std::numeric_limits<FwSizeType>::max() >= std::numeric_limits<PlatformIntType>::max(),
"Range of PlatformIntType does not fit within range of FwSizeType");
PlatformIntType total_needed_size_api = vsnprintf(us, cap, formatString, args);

Check notice

Code scanning / CodeQL

Use of basic integral type Note

total_needed_size_api uses the basic integral type int rather than a typedef with size and signedness.
Svc/ActiveTextLogger/LogFile.cpp
return false;
}

FwSignedSizeType fileSize = 0;

Check notice

Code scanning / CodeQL

Use of basic integral type Note

fileSize uses the basic integral type signed long rather than a typedef with size and signedness.
Svc/FileManager/FileManager.cpp

const int status = system(buffer);
// Call the system
const int status = system(stringBuffer.toChar());

Check notice

Code scanning / CodeQL

Use of basic integral type Note

status uses the basic integral type int rather than a typedef with size and signedness.
github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 29, 2025
View reviewed changes
Fw/Types/StringBase.cpp
}

void StringBase::vformat(const CHAR* formatString, va_list args) {
FormatStatus StringBase::vformat(const CHAR* formatString, va_list args) {

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
@thomas-bc thomas-bc merged commit ad3c2e5 into nasa:devel Jan 29, 2025
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thomas-bc thomas-bc thomas-bc approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@LeStarch @thomas-bc