Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sys/posix/pthread: newlib compatibility #17734

Merged
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
12 changes: 9 additions & 3 deletions sys/posix/pthread/include/pthread_once.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,21 @@ extern "C" {

/**
* @brief Datatype to supply to pthread_once().
* @details This data type must be compatible with the one defined
* in newlib's `include/sys/_pthreadtypes.h`.
*/
typedef volatile int pthread_once_t;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really clear to me why pthread_once_t was declared to be volatile, propably to vaoid optimizing it out. But I don't see any reason why it shouldn't be optimized out.

typedef struct {
int is_initialized; /**< initialized */
Copy link
Contributor

Choose a reason for hiding this comment

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

So this value is only touched by newlib?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A variable of type pthread_once_t can be used by both user code and the gcc/g++ libraries to ensure that the specified function (the init_routine) is executed only once. However, the value of such a pthread_once_t variable must only be touched by the pthread_once() function. The state of a pthread_once_t variable should not be of interest for code usage.

Before a pthread_once_t variable is used, it must be initialized with PTHREAD_ONCE_INIT. Since the variable was previously a simple int variable, it would be theoretically possible for the user code to initialize it with = 0.But that should now trigger a compilation error.

int init_executed; /**< init function executed */
} pthread_once_t;
benpicco marked this conversation as resolved.
Show resolved Hide resolved

/**
* @def PTHREAD_ONCE_INIT
* @brief Initialization for pthread_once_t.
* @details A zeroed out pthread_once_t is initialized.
* @details pthread_once_t variables are declared as initialized, but
* the init function is not yet executed.
*/
#define PTHREAD_ONCE_INIT 0
#define PTHREAD_ONCE_INIT { 1, 0 }

/**
* @brief Helper function that ensures that `init_routine` is called at once.
Expand Down
4 changes: 2 additions & 2 deletions sys/posix/pthread/pthread_once.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@

int pthread_once(pthread_once_t *once_control, void (*init_routine)(void))
{
if (*once_control == PTHREAD_ONCE_INIT) {
if (!once_control->init_executed) {
init_routine();
}

*once_control = PTHREAD_ONCE_INIT + 1;
once_control->init_executed = 1;

return 0;
}