Set the Name of Fprime Tasks on Linux #4428
Conversation
LeStarch
left a comment
LeStarch
left a comment
There was a problem hiding this comment.
A few questions, but otherwise looks good!
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
|
|
||
| int set_task_name(pthread_t thread, const Os::Task::Arguments& arguments) { | ||
| int status = 0; |
Check notice
Code scanning / CodeQL
Use of basic integral type Note
| #if defined(TGT_OS_TYPE_LINUX) && defined(__GLIBC__) && defined(_GNU_SOURCE) && defined(POSIX_THREADS_ENABLE_NAMES) && \ | ||
| POSIX_THREADS_ENABLE_NAMES |
Check notice
Code scanning / CodeQL
Conditional compilation Note
| name[Os::Posix::Task::PosixTaskHandle::PTHREAD_NAME_LENGTH - 1] = '\0'; | ||
| status = pthread_setname_np(thread, name); | ||
| #endif | ||
| return status; |
Check warning
Code scanning / CodeQL
Unchecked function argument Warning
| status = pthread_setname_np(thread, name); | ||
| #endif | ||
| return status; | ||
| } |
Check warning
Code scanning / CodeQL
Unchecked function argument Warning
| // 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 | ||
| // Force safe name usage |
Check notice
Code scanning / CodeQL
Use of basic integral type Note
| // 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 | ||
| // Force safe name usage |
Check notice
Code scanning / CodeQL
Use of basic integral type Note
| // 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 | ||
| // Force safe name usage |
Check notice
Code scanning / CodeQL
Use of basic integral type Note
| //! 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
timcanham
left a comment
timcanham
left a comment
There was a problem hiding this comment.
Minor comments, but LGTM
| typedef void* (*pthread_func_ptr)(void*); | ||
|
|
||
| // Forward declaration | ||
| int set_task_name(pthread_t thread, char* name); |
There was a problem hiding this comment.
Minor: If these functions are used just in this file, I would declare them static
There was a problem hiding this comment.
We could. The other functions in this file that are helpers are not declared static.
| #if defined(POSIX_THREADS_ENABLE_NAMES) && POSIX_THREADS_ENABLE_NAMES | ||
| Fw::StringUtils::string_copy(handle.m_name, arguments.m_name.toChar(), sizeof(handle.m_name)); | ||
| #endif | ||
|
|
Check notice
Code scanning / CodeQL
Conditional compilation Note
| Fw::StringUtils::string_copy(handle.m_name, arguments.m_name.toChar(), sizeof(handle.m_name)); | ||
| #endif | ||
|
|
||
| (void)pthread_attr_destroy(&attributes); |
Check warning
Code scanning / CodeQL
Unchecked return value Warning
4 participants
Change Description
Os::Task for linux will set the thread name to the task's name.
Rationale
When attempting to profile fprime code, it would be beneficial to see which fprime tasks are eating up CPU resources from outside tooling.
Testing/Review Recommendations
The current implementation does not prefix the task name w/ the original process name as 16 characters is rather restrictive.
Future Work
N/A
AI Usage (see policy)
N/A