Skip to content

Commit

Permalink
Fixing review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
LeStarch committed May 22, 2024
1 parent 9ffdee8 commit 4e6645a
Show file tree
Hide file tree
Showing 8 changed files with 13 additions and 180 deletions.
6 changes: 5 additions & 1 deletion Fw/Comp/ActiveComponentBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,9 @@ namespace Fw {
// cast void* back to active component
ActiveComponentBase* component = static_cast<ActiveComponentBase*>(component_pointer);

// Each invocation of this function runs a single stage of the thread lifecycle.
// Each invocation of this function runs a single stage of the thread lifecycle. This has moved the thread
// while loop to the top level such that it can be replaced by something else (e.g. cooperative thread
// dispatcher) and is not intrinsic to this code.
switch (component->m_stage) {
// The first stage the active component triggers the "preamble" call before moving into the dispatching
// stage of the component thread.
Expand Down Expand Up @@ -126,6 +128,8 @@ namespace Fw {
void ActiveComponentBase::s_taskLoop(void* component_pointer) {
FW_ASSERT(component_pointer != nullptr);
ActiveComponentBase* component = static_cast<ActiveComponentBase*>(component_pointer);
// A non-cooperative task switching implementation is just a while-loop around the active component
// state-machine. Here the while loop is at top-level.
while (component->m_stage != ActiveComponentBase::Lifecycle::DONE) {
ActiveComponentBase::s_taskStateMachine(component);
}

Check warning

Code scanning / CodeQL

Unbounded loop Warning

This loop does not have a fixed bound.
Expand Down
46 changes: 0 additions & 46 deletions Os/Baremetal/File.cpp

This file was deleted.

78 changes: 0 additions & 78 deletions Os/Baremetal/Task.cpp

This file was deleted.

2 changes: 0 additions & 2 deletions Os/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,11 @@ if (FPRIME_USE_BAREMETAL_SCHEDULER)
list(REMOVE_ITEM SOURCE_FILES "${ITER_ITEM}")
endif()
endforeach()
list(APPEND SOURCE_FILES "${CMAKE_CURRENT_LIST_DIR}/Baremetal/File.cpp")
list(APPEND SOURCE_FILES "${CMAKE_CURRENT_LIST_DIR}/Baremetal/FileSystem.cpp")
list(APPEND SOURCE_FILES "${CMAKE_CURRENT_LIST_DIR}/Baremetal/IntervalTimer.cpp")
list(APPEND SOURCE_FILES "${CMAKE_CURRENT_LIST_DIR}/Baremetal/Mutex.cpp")
list(APPEND SOURCE_FILES "${CMAKE_CURRENT_LIST_DIR}/Baremetal/Queue.cpp")
list(APPEND SOURCE_FILES "${CMAKE_CURRENT_LIST_DIR}/Baremetal/SystemResources.cpp")
list(APPEND SOURCE_FILES "${CMAKE_CURRENT_LIST_DIR}/Baremetal/Task.cpp")
endif()
register_fprime_module()
require_fprime_implementation(Os/File)
Expand Down
2 changes: 1 addition & 1 deletion Os/Posix/Task.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ namespace Task {
void* pthread_entry_wrapper(void* wrapper_pointer) {
FW_ASSERT(wrapper_pointer != nullptr);
Os::Task::TaskRoutineWrapper& wrapper = *reinterpret_cast<Os::Task::TaskRoutineWrapper*>(wrapper_pointer);
wrapper.run(&wrapper);
wrapper.run(&wrapper);
return nullptr;
}

Expand Down
5 changes: 4 additions & 1 deletion Os/Task.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,10 @@ TaskHandle* Task::getHandle() {
}

FwSizeType Task::getNumTasks() {
return Task::s_numTasks;
Task::s_taskMutex.lock();
FwSizeType num_tasks = Task::s_numTasks;
Task::s_taskMutex.unlock();
return num_tasks;
}

void Task::registerTaskRegistry(TaskRegistry* registry) {
Expand Down
4 changes: 3 additions & 1 deletion Os/Task.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,9 @@ namespace Os {
void onStart() override;

//! \brief invoke the task's routine
//~
//! This will invoke the task's routine passing this as the argument to that call. This is used as a helper when
//! running this task (e.g. repetitive cooperative calls).
void invokeRoutine();

//! \brief join calling thread to this thread
Expand Down Expand Up @@ -339,7 +342,6 @@ namespace Os {
Mutex m_lock; //!< Guards state transitions
TaskRoutineWrapper m_wrapper; //!< Concrete storage for task routine wrapper

PlatformIntType m_identifier = 0; //!< thread independent identifier
bool m_registered = false; //!< Was this task registered

Check notice

Code scanning / CodeQL

Use of basic integral type Note

m_registered uses the basic integral type bool rather than a typedef with size and signedness.

// This section is used to store the implementation-defined file handle. To Os::File and fprime, this type is
Expand Down
50 changes: 0 additions & 50 deletions Os/TaskCommon.cpp

This file was deleted.

0 comments on commit 4e6645a

Please sign in to comment.