pw_thread_threadx: apply pw_thread_embos style changes
Change-Id: I5a22c093df0de2f1e2f0753a58cfc670490973f4
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/45422
Reviewed-by: Keir Mierle <keir@google.com>
Pigweed-Auto-Submit: Ewout van Bekkum <ewout@google.com>
Commit-Queue: Ewout van Bekkum <ewout@google.com>
diff --git a/pw_thread_threadx/BUILD b/pw_thread_threadx/BUILD
index a333d23..f69e160 100644
--- a/pw_thread_threadx/BUILD
+++ b/pw_thread_threadx/BUILD
@@ -65,6 +65,7 @@
deps = [
":id",
"//pw_assert",
+ "//pw_string",
"//pw_thread:thread_headers",
],
# TODO(pwbug/317): This should depend on ThreadX but our third parties
diff --git a/pw_thread_threadx/BUILD.gn b/pw_thread_threadx/BUILD.gn
index f7a090b..79d475a 100644
--- a/pw_thread_threadx/BUILD.gn
+++ b/pw_thread_threadx/BUILD.gn
@@ -104,6 +104,7 @@
public_deps = [
":config",
"$dir_pw_assert",
+ "$dir_pw_string",
"$dir_pw_third_party/threadx",
"$dir_pw_thread:id",
"$dir_pw_thread:thread.facade",
diff --git a/pw_thread_threadx/public/pw_thread_threadx/config.h b/pw_thread_threadx/public/pw_thread_threadx/config.h
index a47e1eb..14405e2 100644
--- a/pw_thread_threadx/public/pw_thread_threadx/config.h
+++ b/pw_thread_threadx/public/pw_thread_threadx/config.h
@@ -17,8 +17,16 @@
#include "tx_api.h"
// Whether thread joining is enabled. By default this is disabled.
-// When enabled this adds a TX_EVENT_FLAGS_GROUP to every pw::thread::Thread's
-// context.
+//
+// We suggest only enabling this when thread joining is required to minimize
+// the RAM and ROM cost of threads.
+//
+// Enabling this grows the RAM footprint of every pw::thread::Thread as it adds
+// a TX_EVENT_FLAGS_GROUP to every thread's pw::thread::threadx::Context. In
+// addition, there is a minute ROM cost to construct and destroy this added
+// object.
+//
+// PW_THREAD_JOINING_ENABLED gets set to this value.
#ifndef PW_THREAD_THREADX_CONFIG_JOINING_ENABLED
#define PW_THREAD_THREADX_CONFIG_JOINING_ENABLED 0
#endif // PW_THREAD_THREADX_CONFIG_JOINING_ENABLED
@@ -28,7 +36,7 @@
// stack size.
#ifndef PW_THREAD_THREADX_CONFIG_DEFAULT_STACK_SIZE_WORDS
#define PW_THREAD_THREADX_CONFIG_DEFAULT_STACK_SIZE_WORDS \
- TX_MINIMUM_STACK / sizeof(ULONG)
+ (TX_MINIMUM_STACK / sizeof(ULONG))
#endif // PW_THREAD_THREADX_CONFIG_DEFAULT_STACK_SIZE_WORDS
// The maximum length of a thread's name, not including null termination. By
@@ -48,7 +56,7 @@
// The minimum priority level, this is normally based on the number of priority
// levels.
#ifndef PW_THREAD_THREADX_CONFIG_MIN_PRIORITY
-#define PW_THREAD_THREADX_CONFIG_MIN_PRIORITY TX_MAX_PRIORITIES - 1
+#define PW_THREAD_THREADX_CONFIG_MIN_PRIORITY (TX_MAX_PRIORITIES - 1)
#endif // PW_THREAD_THREADX_CONFIG_MIN_PRIORITY
// The default priority level. By default this uses the minimal ThreadX
@@ -61,7 +69,7 @@
namespace pw::thread::threadx::config {
inline constexpr size_t kMaximumNameLength =
- PW_THREAD_THREADX_CONFIG_MAX_THREAD_NAME_LEN + 1;
+ PW_THREAD_THREADX_CONFIG_MAX_THREAD_NAME_LEN;
inline constexpr size_t kMinimumStackSizeWords =
TX_MINIMUM_STACK / sizeof(ULONG);
inline constexpr size_t kDefaultStackSizeWords =
diff --git a/pw_thread_threadx/public/pw_thread_threadx/context.h b/pw_thread_threadx/public/pw_thread_threadx/context.h
index 162e3b3..4d68ac5 100644
--- a/pw_thread_threadx/public/pw_thread_threadx/context.h
+++ b/pw_thread_threadx/public/pw_thread_threadx/context.h
@@ -17,6 +17,7 @@
#include <cstring>
#include <span>
+#include "pw_string/util.h"
#include "pw_thread_threadx/config.h"
#include "tx_api.h"
#include "tx_thread.h"
@@ -63,17 +64,12 @@
void set_in_use(bool in_use = true) { in_use_ = in_use; }
const char* name() const { return name_.data(); }
- void set_name(const char* name) {
- strncpy(name_.data(), name, name_.size() - 1);
- // Forcefully NULL terminate as strncpy does not NULL terminate if the count
- // limit is hit.
- name_[name_.size() - 1] = '\0';
- }
+ void set_name(const char* name) { string::Copy(name, name_); }
using ThreadRoutine = void (*)(void* arg);
void set_thread_routine(ThreadRoutine entry, void* arg) {
- entry_ = entry;
- arg_ = arg;
+ user_thread_entry_function_ = entry;
+ user_thread_entry_arg_ = arg;
}
bool detached() const { return detached_; }
@@ -86,14 +82,14 @@
TX_EVENT_FLAGS_GROUP& join_event_group() { return event_group_; }
#endif // PW_THREAD_JOINING_ENABLED
- static void RunThread(ULONG void_context_ptr);
+ static void ThreadEntryPoint(ULONG void_context_ptr);
static void DeleteThread(Context& context);
TX_THREAD tcb_;
std::span<ULONG> stack_span_;
- ThreadRoutine entry_ = nullptr;
- void* arg_ = nullptr;
+ ThreadRoutine user_thread_entry_function_ = nullptr;
+ void* user_thread_entry_arg_ = nullptr;
#if PW_THREAD_JOINING_ENABLED
// Note that the ThreadX life cycle of this event group is managed together
// with the thread life cycle, not this object's life cycle.
@@ -104,8 +100,8 @@
bool thread_done_ = false;
// The TCB does not have storage for the name, ergo we provide storage for
- // the thread's name which can be truncated down to just a null delimeter.
- std::array<char, config::kMaximumNameLength> name_;
+ // the thread's name which can be truncated down to just a null delimiter.
+ std::array<char, config::kMaximumNameLength + 1> name_;
};
// Static thread context allocation including the stack along with the Context.
diff --git a/pw_thread_threadx/thread.cc b/pw_thread_threadx/thread.cc
index b3ed29a..673a3ca 100644
--- a/pw_thread_threadx/thread.cc
+++ b/pw_thread_threadx/thread.cc
@@ -30,9 +30,11 @@
#endif // PW_THREAD_JOINING_ENABLED
} // namespace
-void Context::RunThread(ULONG void_context_ptr) {
+void Context::ThreadEntryPoint(ULONG void_context_ptr) {
Context& context = *reinterpret_cast<Context*>(void_context_ptr);
- context.entry_(context.arg_);
+
+ // Invoke the user's thread function. This may never return.
+ context.user_thread_entry_function_(context.user_thread_entry_arg_);
// Raise our preemption threshold as a thread only critical section to guard
// against join() and detach().
@@ -50,8 +52,9 @@
context.set_in_use(false);
#if PW_THREAD_JOINING_ENABLED
- // Just in case someone abused our API, ensure their use of the event group
- // is properly handled by the kernel regardless.
+ // If the thread handle was detached before the thread finished execution,
+ // i.e. got here, then we are responsible for cleaning up the join event
+ // group.
const UINT event_group_result =
tx_event_flags_delete(&context.join_event_group());
PW_DCHECK_UINT_EQ(TX_SUCCESS,
@@ -144,7 +147,7 @@
const UINT thread_result =
tx_thread_create(&options.context()->tcb(),
const_cast<char*>(native_type_->name()),
- Context::RunThread,
+ Context::ThreadEntryPoint,
reinterpret_cast<ULONG>(native_type_),
options.context()->stack().data(),
options.context()->stack().size_bytes(),
@@ -164,12 +167,12 @@
tx_thread_resume(&native_type_->tcb());
if (thread_done) {
- // The task finished (hit end of Context::RunThread) before we invoked
- // detach, clean up the thread.
+ // The task finished (hit end of Context::ThreadEntryPoint) before we
+ // invoked detach, clean up the thread.
Context::DeleteThread(*native_type_);
} else {
// We're detaching before the task finished, defer cleanup to the task at
- // the end of Context::RunThread.
+ // the end of Context::ThreadEntryPoint.
}
// Update to no longer represent a thread of execution.
