Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/actions/spelling/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,7 @@ serializables
setaffinity
setinheritsched
SETLOGGING
setname
setprotocol
setschedparam
setschedpolicy
Expand Down
27 changes: 27 additions & 0 deletions Os/Posix/Task.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include "Fw/Logger/Logger.hpp"
#include "Fw/Types/Assert.hpp"
#include "Fw/Types/StringUtils.hpp"
#include "Os/Posix/Task.hpp"
#include "Os/Posix/error.hpp"
#include "Os/Task.hpp"
Expand All @@ -22,9 +23,17 @@

typedef void* (*pthread_func_ptr)(void*);

// Forward declaration
int set_task_name(pthread_t thread, char* name);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: If these functions are used just in this file, I would declare them static

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could. The other functions in this file that are helpers are not declared static.


void* pthread_entry_wrapper(void* wrapper_pointer) {
FW_ASSERT(wrapper_pointer != nullptr);
// Both downcasts are safe because we know the types
Os::Task::TaskRoutineWrapper& wrapper = *reinterpret_cast<Os::Task::TaskRoutineWrapper*>(wrapper_pointer);
auto handle = reinterpret_cast<Os::Posix::Task::PosixTaskHandle*>(wrapper.m_task.getHandle());
FW_ASSERT(handle != nullptr);
// Task name is on a best effort basis
(void)set_task_name(handle->m_task_descriptor, handle->m_name);
wrapper.run(&wrapper);
return nullptr;
}
Expand Down Expand Up @@ -108,13 +117,27 @@
// According to the man-page this function sets errno rather than returning an error status like other functions
status = pthread_attr_setaffinity_np(&attributes, sizeof(cpu_set_t), &cpu_set);
status = (status == PosixTaskHandle::SUCCESS) ? status : errno;
#else
Fw::Logger::log("[WARNING] %s setting CPU affinity is only available with GNU pthreads\n",
const_cast<CHAR*>(arguments.m_name.toChar()));
#endif
return status;
}

int set_task_name(pthread_t thread, char* name) {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

set_task_name uses the basic integral type int rather than a typedef with size and signedness.

Check notice

Code scanning / CodeQL

Use of basic integral type Note

thread uses the basic integral type unsigned long rather than a typedef with size and signedness.

Check notice

Code scanning / CodeQL

Use of basic integral type Note

name uses the basic integral type char rather than a typedef with size and signedness.
int status = 0;

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.
// pthread_setname_np is a non-POSIX function.
// Limit its use to builds that involve glibc, on Linux, with _GNU_SOURCE defined.
// That's the circumstance in which we expect this feature to work.
#if defined(TGT_OS_TYPE_LINUX) && defined(__GLIBC__) && defined(_GNU_SOURCE) && defined(POSIX_THREADS_ENABLE_NAMES) && \
POSIX_THREADS_ENABLE_NAMES
Comment on lines +132 to +133

Check notice

Code scanning / CodeQL

Conditional compilation Note

Use of conditional compilation must be kept to a minimum.
// Force safe name usage
name[Os::Posix::Task::PosixTaskHandle::PTHREAD_NAME_LENGTH - 1] = '\0';

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter name has not been checked.
status = pthread_setname_np(thread, name);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter thread has not been checked.
#endif
return status;
}

Os::Task::Status PosixTask::create(const Os::Task::Arguments& arguments,
const PosixTask::PermissionExpectation permissions) {
int pthread_status = PosixTaskHandle::SUCCESS;
Expand Down Expand Up @@ -145,6 +168,10 @@
handle.m_is_valid = true;
}

#if defined(POSIX_THREADS_ENABLE_NAMES) && POSIX_THREADS_ENABLE_NAMES

Check notice

Code scanning / CodeQL

Conditional compilation Note

Use of conditional compilation must be kept to a minimum.
Fw::StringUtils::string_copy(handle.m_name, arguments.m_name.toChar(), sizeof(handle.m_name));

Check warning

Code scanning / CodeQL

Unchecked return value Warning

The return value of non-void function
string_copy
is not checked.
#endif

(void)pthread_attr_destroy(&attributes);
return Posix::posix_status_to_task_status(pthread_status);
}
Expand Down
4 changes: 4 additions & 0 deletions Os/Posix/Task.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,16 @@
//! TaskHandle class definition for posix implementations.
//!
struct PosixTaskHandle : public TaskHandle {
static constexpr FwSizeType PTHREAD_NAME_LENGTH = 16; //!< Length of pthread name
static constexpr int SUCCESS = 0;

//! Posix task descriptor
pthread_t m_task_descriptor;
//! Is the above descriptor valid
bool m_is_valid = false;
#if defined(POSIX_THREADS_ENABLE_NAMES) && POSIX_THREADS_ENABLE_NAMES
char m_name[PosixTaskHandle::PTHREAD_NAME_LENGTH];

Check notice

Code scanning / CodeQL

Use of basic integral type Note

m_name uses the basic integral type char rather than a typedef with size and signedness.
#endif
};

//! Posix task implementation as driven by pthreads implementation
Expand Down
6 changes: 6 additions & 0 deletions default/config/FpConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,12 @@ extern "C" {
#define FW_AMPCS_COMPATIBLE 0 //!< Whether or not JPL AMPCS ground system support is enabled.
#endif

// Posix thread names are limited to 16 characters, this can lead to collisions. In the event of a
// collision, set this to 0.
#ifndef POSIX_THREADS_ENABLE_NAMES
#define POSIX_THREADS_ENABLE_NAMES 1 //!< Enable/Disable assigning names to threads
#endif

// *** NOTE configuration checks are in Fw/Cfg/ConfigCheck.cpp in order to have
// the type definitions in Fw/Types/BasicTypes available.
#ifdef __cplusplus
Expand Down
2 changes: 1 addition & 1 deletion default/config/PlatformCfg.fpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
constant FW_CONSOLE_HANDLE_MAX_SIZE = 24

@ Maximum size of a handle for Os::Task
constant FW_TASK_HANDLE_MAX_SIZE = 24
constant FW_TASK_HANDLE_MAX_SIZE = 40

@ Maximum size of a handle for Os::File
constant FW_FILE_HANDLE_MAX_SIZE = 16
Expand Down
Loading