Skip to content

Commit 02afd01

Browse files
m-aleemthomas-bc
andauthored
Fix warnings reported by clang-tidy 20.1 (#3949)
* Clang 20.1 UT Errors * CI fixes * Remove m_rng and use STest * 1 more update to STest * Resolve spellcheck conflict * Resolve spellcheck conflict * Init severityString to null ptr * Fix CPP formatting * Format QueueRules.cpp --------- Co-authored-by: thomas-bc <[email protected]> Co-authored-by: Thomas Boyer-Chammard <[email protected]>
1 parent 60e7cb7 commit 02afd01

File tree

17 files changed

+70
-54
lines changed

17 files changed

+70
-54
lines changed

.github/actions/spelling/expect.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,8 @@ ncsl
417417
newtio
418418
nmsgs
419419
NOBLOCK
420+
NOLINT
421+
NOLINTNEXTLINE
420422
noparent
421423
norecords
422424
NOSPEC
@@ -439,6 +441,7 @@ openmct
439441
openpyxl
440442
openssldir
441443
optarg
444+
optin
442445
optind
443446
orgslist
444447
ortega

Os/Posix/test/ut/PosixConsoleTests.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,9 @@ TEST(Nominal, SwitchStream) {
1919
}
2020
TEST(OffNominal, SwitchStream) {
2121
Os::Posix::Console::PosixConsole posix_console;
22-
ASSERT_DEATH(posix_console.setOutputStream(static_cast<Os::Posix::Console::PosixConsole::Stream>(3)),
23-
"Posix/|\\Console.cpp:33");
22+
ASSERT_DEATH(
23+
posix_console.setOutputStream(static_cast<Os::Posix::Console::PosixConsole::Stream>(3)),
24+
"Posix/|\\Console.cpp:33"); // NOLINT(clang-analyzer-optin.core.EnumCastOutOfRange) intentional death test
2425
}
2526

2627
int main(int argc, char** argv) {

Os/Posix/test/ut/PosixDirectoryTests.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@
1010
#include "STest/Pick/Pick.hpp"
1111
#include "STest/Scenario/Scenario.hpp"
1212

13-
#include <fcntl.h> // for ::open()
13+
#include <fcntl.h> // for ::open()
14+
#include <unistd.h> // for ::close()
1415

1516
namespace Os {
1617
namespace Test {
@@ -34,7 +35,10 @@ void setUp(Os::Test::Directory::Tester* tester) {
3435
tester->m_filenames.push_back(FILENAME_PREFIX + std::to_string(i));
3536
}
3637
for (auto filename : tester->m_filenames) {
37-
::open((tester->m_path + "/" + filename).c_str(), O_CREAT);
38+
int fd = ::open((tester->m_path + "/" + filename).c_str(), O_CREAT | O_WRONLY, 0644);
39+
if (fd >= 0) {
40+
::close(fd);
41+
}
3842
}
3943
}
4044

Os/test/ut/queue/QueueRules.cpp

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,21 @@
33
// \brief queue rule implementations
44
// ======================================================================
55

6+
#include <memory>
67
#include "CommonTests.hpp"
78
#include "Fw/Types/String.hpp"
89

910
struct PickedMessage {
1011
FwSizeType size;
1112
FwQueuePriorityType priority;
12-
U8* sent;
13+
std::unique_ptr<U8[]> sent;
1314
U8 received[QUEUE_MESSAGE_SIZE_UPPER_BOUND];
15+
16+
// Constructor
17+
PickedMessage() : size(0), priority(0), sent(nullptr) {}
18+
19+
// Get raw pointer for API compatibility
20+
U8* get_sent() const { return sent.get(); }
1421
};
1522

1623
PickedMessage pick_message(FwSizeType max_size) {
@@ -20,7 +27,7 @@ PickedMessage pick_message(FwSizeType max_size) {
2027
// Force priority to be in a smaller range to produce more same-priority messages
2128
message.priority = STest::Random::lowerUpper(0, std::numeric_limits<I8>::max());
2229

23-
message.sent = new U8[message.size];
30+
message.sent.reset(new U8[message.size]);
2431
for (FwSizeType i = 0; i < message.size; i++) {
2532
message.sent[i] = STest::Random::lowerUpper(0, std::numeric_limits<U8>::max());
2633
}
@@ -81,9 +88,8 @@ void Os::Test::Queue::Tester::SendNotFull::action(Os::Test::Queue::Tester& state
8188
// Prevent lock-up
8289
ASSERT_LT(state.queue.getMessagesAvailable(), state.queue.getDepth());
8390
ASSERT_FALSE(state.is_shadow_full());
84-
QueueInterface::Status status = state.shadow_send(pick.sent, pick.size, pick.priority, blocking);
85-
QueueInterface::Status test_status = state.queue.send(pick.sent, pick.size, pick.priority, blocking);
86-
delete[] pick.sent; // Clean-up
91+
QueueInterface::Status status = state.shadow_send(pick.get_sent(), pick.size, pick.priority, blocking);
92+
QueueInterface::Status test_status = state.queue.send(pick.get_sent(), pick.size, pick.priority, blocking);
8793
ASSERT_EQ(status, QueueInterface::Status::OP_OK);
8894
ASSERT_EQ(test_status, status);
8995
if (this->m_end_check) {
@@ -106,10 +112,10 @@ bool Os::Test::Queue::Tester::SendFullNoBlock::precondition(const Os::Test::Queu
106112
void Os::Test::Queue::Tester::SendFullNoBlock::action(Os::Test::Queue::Tester& state //!< The test state
107113
) {
108114
PickedMessage pick = pick_message(state.shadow.messageSize);
109-
QueueInterface::Status status = state.shadow_send(pick.sent, pick.size, pick.priority, QueueInterface::NONBLOCKING);
115+
QueueInterface::Status status =
116+
state.shadow_send(pick.get_sent(), pick.size, pick.priority, QueueInterface::NONBLOCKING);
110117
QueueInterface::Status test_status =
111-
state.queue.send(pick.sent, pick.size, pick.priority, QueueInterface::NONBLOCKING);
112-
delete[] pick.sent;
118+
state.queue.send(pick.get_sent(), pick.size, pick.priority, QueueInterface::NONBLOCKING);
113119

114120
ASSERT_EQ(status, QueueInterface::Status::FULL);
115121
ASSERT_EQ(test_status, status);
@@ -199,19 +205,17 @@ void Os::Test::Queue::Tester::Overflow::action(Os::Test::Queue::Tester& state /
199205
while (not state.is_shadow_full()) {
200206
PickedMessage pick = pick_message(state.shadow.messageSize);
201207
QueueInterface::Status status =
202-
state.shadow_send(pick.sent, pick.size, pick.priority, QueueInterface::BlockingType::NONBLOCKING);
208+
state.shadow_send(pick.get_sent(), pick.size, pick.priority, QueueInterface::BlockingType::NONBLOCKING);
203209
QueueInterface::Status test_status =
204-
state.queue.send(pick.sent, pick.size, pick.priority, QueueInterface::BlockingType::NONBLOCKING);
205-
delete[] pick.sent;
210+
state.queue.send(pick.get_sent(), pick.size, pick.priority, QueueInterface::BlockingType::NONBLOCKING);
206211
ASSERT_EQ(status, QueueInterface::Status::OP_OK);
207212
ASSERT_EQ(status, test_status);
208213
}
209214
PickedMessage pick = pick_message(state.shadow.messageSize);
210215
QueueInterface::Status status =
211-
state.shadow_send(pick.sent, pick.size, pick.priority, QueueInterface::BlockingType::NONBLOCKING);
216+
state.shadow_send(pick.get_sent(), pick.size, pick.priority, QueueInterface::BlockingType::NONBLOCKING);
212217
QueueInterface::Status test_status =
213-
state.queue.send(pick.sent, pick.size, pick.priority, QueueInterface::BlockingType::NONBLOCKING);
214-
delete[] pick.sent;
218+
state.queue.send(pick.get_sent(), pick.size, pick.priority, QueueInterface::BlockingType::NONBLOCKING);
215219
ASSERT_EQ(status, QueueInterface::Status::FULL);
216220
ASSERT_EQ(status, test_status);
217221
state.shadow_check();
@@ -280,16 +284,15 @@ void Os::Test::Queue::Tester::SendBlock::action(Os::Test::Queue::Tester& state
280284
PickedMessage pick = pick_message(state.shadow.messageSize);
281285
this->notify_other("SendUnblock");
282286
QueueInterface::Status status =
283-
state.shadow_send(pick.sent, pick.size, pick.priority, QueueInterface::BlockingType::BLOCKING);
287+
state.shadow_send(pick.get_sent(), pick.size, pick.priority, QueueInterface::BlockingType::BLOCKING);
284288
getLock().unlock();
285289
QueueInterface::Status test_status =
286-
state.queue.send(pick.sent, pick.size, pick.priority, QueueInterface::BlockingType::BLOCKING);
290+
state.queue.send(pick.get_sent(), pick.size, pick.priority, QueueInterface::BlockingType::BLOCKING);
287291
getLock().lock();
288292
// Condition should be set after block
289293
ASSERT_TRUE(this->getCondition());
290294
// Unblock the shadow queue send
291295
state.shadow_send_unblock();
292-
delete[] pick.sent;
293296

294297
ASSERT_EQ(status, QueueInterface::Status::OP_OK);
295298
ASSERT_EQ(test_status, status);

Svc/ActiveTextLogger/ActiveTextLogger.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ namespace Svc {
4545

4646
// Format the string here, so that it is done in the task context
4747
// of the caller. Format doc borrowed from PassiveTextLogger.
48-
const char *severityString = "UNKNOWN";
48+
const char *severityString = nullptr;
4949
switch (severity.e) {
5050
case Fw::LogSeverity::FATAL:
5151
severityString = "FATAL";

Svc/BufferManager/CMakeLists.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ set(SOURCE_FILES
1515
register_fprime_module()
1616

1717
### UTS ###
18+
set(UT_MOD_DEPS
19+
STest
20+
)
1821
set(UT_SOURCE_FILES
1922
"${FPRIME_FRAMEWORK_PATH}/Svc/BufferManager/BufferManager.fpp"
2023
"${CMAKE_CURRENT_LIST_DIR}/test/ut/BufferManagerTester.cpp"

Svc/BufferManager/test/ut/BufferManagerTester.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -235,15 +235,12 @@ namespace Svc {
235235
REQUIREMENT("FPRIME-BM-006");
236236

237237
// randomly return buffers
238-
time_t t;
239-
srand(static_cast<unsigned>(time(&t)));
240-
241238
bool returned[BIN1_NUM_BUFFERS] = {false};
242239

243240
for (U16 b=0; b<BIN1_NUM_BUFFERS; b++) {
244241
U16 entry;
245242
while (true) {
246-
entry = rand() % BIN1_NUM_BUFFERS;
243+
entry = STest::Pick::lowerUpper(0, BIN1_NUM_BUFFERS - 1);
247244
if (not returned[entry]) {
248245
returned[entry] = true;
249246
break;

Svc/BufferManager/test/ut/BufferManagerTester.hpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
#include "BufferManagerGTestBase.hpp"
1717
#include "Svc/BufferManager/BufferManagerComponentImpl.hpp"
18+
#include <STest/Pick/Pick.hpp>
1819

1920
namespace Svc {
2021

@@ -76,6 +77,8 @@ namespace Svc {
7677
//!
7778
BufferManagerComponentImpl component;
7879

80+
81+
7982
void textLogIn(
8083
const FwEventIdType id, //!< The event ID
8184
const Fw::Time& timeTag, //!< The time

Svc/CmdSequencer/test/ut/SequenceFiles/BadDescriptorFile.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ namespace Svc {
5151
Fw::Time t(TimeBase::TB_WORKSTATION_TIME, 0, 0);
5252
// Force an invalid record descriptor
5353
FPrime::Records::Descriptor descriptor =
54-
static_cast<FPrime::Records::Descriptor>(10);
54+
static_cast<FPrime::Records::Descriptor>(10); // NOLINT(clang-analyzer-optin.core.EnumCastOutOfRange) intentional test
5555
FPrime::Records::serialize(
5656
descriptor,
5757
t,
@@ -73,7 +73,7 @@ namespace Svc {
7373
for (U32 i = 0; i < this->n; ++i) {
7474
// Force an invalid time flag
7575
const AMPCSSequence::Record::TimeFlag::t timeFlag =
76-
static_cast<AMPCSSequence::Record::TimeFlag::t>(10);
76+
static_cast<AMPCSSequence::Record::TimeFlag::t>(10); // NOLINT(clang-analyzer-optin.core.EnumCastOutOfRange) intentional test
7777
const AMPCSSequence::Record::Time::t time = 0;
7878
const AMPCSSequence::Record::Opcode::t opcode = i;
7979
const U32 argument = i + 1;

Svc/DpCatalog/CMakeLists.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ set(HEADER_FILES
2020

2121
register_fprime_module()
2222
### UTs ###
23+
set(UT_MOD_DEPS
24+
STest
25+
)
2326
set(UT_SOURCE_FILES
2427
"${CMAKE_CURRENT_LIST_DIR}/DpCatalog.fpp"
2528
"${CMAKE_CURRENT_LIST_DIR}/test/ut/DpCatalogTester.cpp"

0 commit comments

Comments
 (0)