Skip to content

posix + tests: correct timing functions to use CLOCK_REALTIME #88762

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

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
5 changes: 0 additions & 5 deletions include/zephyr/posix/time.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,6 @@ extern "C" {
#define TIMER_ABSTIME 4
#endif

static inline int32_t _ts_to_ms(const struct timespec *to)
{
return (int32_t)(to->tv_sec * MSEC_PER_SEC) + (int32_t)(to->tv_nsec / NSEC_PER_MSEC);
}

int clock_gettime(clockid_t clock_id, struct timespec *ts);
int clock_getres(clockid_t clock_id, struct timespec *ts);
int clock_settime(clockid_t clock_id, const struct timespec *ts);
Expand Down
105 changes: 77 additions & 28 deletions lib/posix/options/cond.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

#include "posix_clock.h"
#include "posix_internal.h"

#include <zephyr/init.h>
Expand All @@ -15,13 +16,13 @@

LOG_MODULE_REGISTER(pthread_cond, CONFIG_PTHREAD_COND_LOG_LEVEL);

int64_t timespec_to_timeoutms(const struct timespec *abstime);

__pinned_bss
static struct k_condvar posix_cond_pool[CONFIG_MAX_PTHREAD_COND_COUNT];
static __pinned_bss struct posix_cond posix_cond_pool[CONFIG_MAX_PTHREAD_COND_COUNT];

SYS_BITARRAY_DEFINE_STATIC(posix_cond_bitarray, CONFIG_MAX_PTHREAD_COND_COUNT);

BUILD_ASSERT(sizeof(struct posix_condattr) <= sizeof(pthread_condattr_t),
"posix_condattr is too large");

/*
* We reserve the MSB to mark a pthread_cond_t as initialized (from the
* perspective of the application). With a linear space, this means that
Expand All @@ -30,7 +31,7 @@ SYS_BITARRAY_DEFINE_STATIC(posix_cond_bitarray, CONFIG_MAX_PTHREAD_COND_COUNT);
BUILD_ASSERT(CONFIG_MAX_PTHREAD_COND_COUNT < PTHREAD_OBJ_MASK_INIT,
"CONFIG_MAX_PTHREAD_COND_COUNT is too high");

static inline size_t posix_cond_to_offset(struct k_condvar *cv)
static inline size_t posix_cond_to_offset(struct posix_cond *cv)
{
return cv - posix_cond_pool;
}
Expand All @@ -40,7 +41,7 @@ static inline size_t to_posix_cond_idx(pthread_cond_t cond)
return mark_pthread_obj_uninitialized(cond);
}

static struct k_condvar *get_posix_cond(pthread_cond_t cond)
static struct posix_cond *get_posix_cond(pthread_cond_t cond)
{
int actually_initialized;
size_t bit = to_posix_cond_idx(cond);
Expand All @@ -66,10 +67,10 @@ static struct k_condvar *get_posix_cond(pthread_cond_t cond)
return &posix_cond_pool[bit];
}

static struct k_condvar *to_posix_cond(pthread_cond_t *cvar)
static struct posix_cond *to_posix_cond(pthread_cond_t *cvar)
{
size_t bit;
struct k_condvar *cv;
struct posix_cond *cv;

if (*cvar != PTHREAD_COND_INITIALIZER) {
return get_posix_cond(*cvar);
Expand All @@ -85,24 +86,30 @@ static struct k_condvar *to_posix_cond(pthread_cond_t *cvar)
/* Record the associated posix_cond in mu and mark as initialized */
*cvar = mark_pthread_obj_initialized(bit);
cv = &posix_cond_pool[bit];
(void)pthread_condattr_init((pthread_condattr_t *)&cv->attr);

return cv;
}

static int cond_wait(pthread_cond_t *cond, pthread_mutex_t *mu, k_timeout_t timeout)
static int cond_wait(pthread_cond_t *cond, pthread_mutex_t *mu, const struct timespec *abstime)
{
int ret;
struct k_mutex *m;
struct k_condvar *cv;
struct posix_cond *cv;
k_timeout_t timeout = K_FOREVER;

m = to_posix_mutex(mu);
cv = to_posix_cond(cond);
if (cv == NULL || m == NULL) {
return EINVAL;
}

if (abstime != NULL) {
timeout = K_MSEC(timespec_to_timeoutms(cv->attr.clock, abstime));
}

LOG_DBG("Waiting on cond %p with timeout %llx", cv, timeout.ticks);
ret = k_condvar_wait(cv, m, timeout);
ret = k_condvar_wait(&cv->condvar, m, timeout);
if (ret == -EAGAIN) {
LOG_DBG("Timeout waiting on cond %p", cv);
ret = ETIMEDOUT;
Expand All @@ -120,15 +127,15 @@ static int cond_wait(pthread_cond_t *cond, pthread_mutex_t *mu, k_timeout_t time
int pthread_cond_signal(pthread_cond_t *cvar)
{
int ret;
struct k_condvar *cv;
struct posix_cond *cv;

cv = to_posix_cond(cvar);
if (cv == NULL) {
return EINVAL;
}

LOG_DBG("Signaling cond %p", cv);
ret = k_condvar_signal(cv);
ret = k_condvar_signal(&cv->condvar);
if (ret < 0) {
LOG_DBG("k_condvar_signal() failed: %d", ret);
return -ret;
Expand All @@ -142,15 +149,15 @@ int pthread_cond_signal(pthread_cond_t *cvar)
int pthread_cond_broadcast(pthread_cond_t *cvar)
{
int ret;
struct k_condvar *cv;
struct posix_cond *cv;

cv = get_posix_cond(*cvar);
if (cv == NULL) {
return EINVAL;
}

LOG_DBG("Broadcasting on cond %p", cv);
ret = k_condvar_broadcast(cv);
ret = k_condvar_broadcast(&cv->condvar);
if (ret < 0) {
LOG_DBG("k_condvar_broadcast() failed: %d", ret);
return -ret;
Expand All @@ -163,27 +170,39 @@ int pthread_cond_broadcast(pthread_cond_t *cvar)

int pthread_cond_wait(pthread_cond_t *cv, pthread_mutex_t *mut)
{
return cond_wait(cv, mut, K_FOREVER);
return cond_wait(cv, mut, NULL);
}

int pthread_cond_timedwait(pthread_cond_t *cv, pthread_mutex_t *mut, const struct timespec *abstime)
{
return cond_wait(cv, mut, K_MSEC((int32_t)timespec_to_timeoutms(abstime)));
if ((abstime == NULL) || !timespec_is_valid(abstime)) {
LOG_DBG("%s is invalid", "abstime");
return EINVAL;
}

return cond_wait(cv, mut, abstime);
}

int pthread_cond_init(pthread_cond_t *cvar, const pthread_condattr_t *att)
{
struct k_condvar *cv;
struct posix_cond *cv;
struct posix_condattr *attr = (struct posix_condattr *)attr;
Copy link
Preview

Copilot AI Apr 25, 2025

Choose a reason for hiding this comment

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

The local variable 'attr' is incorrectly initialized by casting itself instead of the function parameter. Change it to 'struct posix_condattr *attr = (struct posix_condattr *)att;' to correctly reference the passed attribute.

Suggested change
struct posix_condattr *attr = (struct posix_condattr *)attr;
struct posix_condattr *attr = (struct posix_condattr *)att;

Copilot uses AI. Check for mistakes.

Copy link
Contributor

Choose a reason for hiding this comment

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

#89076 :)


ARG_UNUSED(att);
*cvar = PTHREAD_COND_INITIALIZER;

/* calls k_condvar_init() */
cv = to_posix_cond(cvar);
if (cv == NULL) {
return ENOMEM;
}

if (attr != NULL) {
if (!attr->initialized) {
return EINVAL;
}

(void)pthread_condattr_destroy((pthread_condattr_t *)&cv->attr);
cv->attr = *attr;
}

LOG_DBG("Initialized cond %p", cv);

return 0;
Expand All @@ -193,7 +212,7 @@ int pthread_cond_destroy(pthread_cond_t *cvar)
{
int err;
size_t bit;
struct k_condvar *cv;
struct posix_cond *cv;

cv = get_posix_cond(*cvar);
if (cv == NULL) {
Expand All @@ -218,7 +237,7 @@ static int pthread_cond_pool_init(void)
size_t i;

for (i = 0; i < CONFIG_MAX_PTHREAD_COND_COUNT; ++i) {
err = k_condvar_init(&posix_cond_pool[i]);
err = k_condvar_init(&posix_cond_pool[i].condvar);
__ASSERT_NO_MSG(err == 0);
}

Expand All @@ -227,35 +246,65 @@ static int pthread_cond_pool_init(void)

int pthread_condattr_init(pthread_condattr_t *att)
{
__ASSERT_NO_MSG(att != NULL);
struct posix_condattr *const attr = (struct posix_condattr *)att;

if (att == NULL) {
return EINVAL;
}
if (attr->initialized) {
LOG_DBG("%s %s initialized", "attribute", "already");
return EINVAL;
}

att->clock = CLOCK_MONOTONIC;
attr->clock = CLOCK_REALTIME;
attr->initialized = true;

return 0;
}

int pthread_condattr_destroy(pthread_condattr_t *att)
{
ARG_UNUSED(att);
struct posix_condattr *const attr = (struct posix_condattr *)att;

if ((attr == NULL) || !attr->initialized) {
LOG_DBG("%s %s initialized", "attribute", "not");
return EINVAL;
}

*attr = (struct posix_condattr){0};

return 0;
}

int pthread_condattr_getclock(const pthread_condattr_t *ZRESTRICT att,
clockid_t *ZRESTRICT clock_id)
{
*clock_id = att->clock;
struct posix_condattr *const attr = (struct posix_condattr *)att;

if ((attr == NULL) || !attr->initialized) {
LOG_DBG("%s %s initialized", "attribute", "not");
return EINVAL;
}

*clock_id = attr->clock;

return 0;
}

int pthread_condattr_setclock(pthread_condattr_t *att, clockid_t clock_id)
{
struct posix_condattr *const attr = (struct posix_condattr *)att;

if (clock_id != CLOCK_REALTIME && clock_id != CLOCK_MONOTONIC) {
return -EINVAL;
}

att->clock = clock_id;
if ((attr == NULL) || !attr->initialized) {
LOG_DBG("%s %s initialized", "attribute", "not");
return EINVAL;
}

attr->clock = clock_id;

return 0;
}
Expand Down
22 changes: 17 additions & 5 deletions lib/posix/options/mqueue.c
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
*
* SPDX-License-Identifier: Apache-2.0
*/

#include "posix_clock.h"

#include <zephyr/kernel.h>
#include <errno.h>
#include <string.h>
Expand Down Expand Up @@ -34,7 +37,6 @@ K_SEM_DEFINE(mq_sem, 1, 1);
/* Initialize the list */
sys_slist_t mq_list = SYS_SLIST_STATIC_INIT(&mq_list);

int64_t timespec_to_timeoutms(const struct timespec *abstime);
static mqueue_object *find_in_list(const char *name);
static int32_t send_message(mqueue_desc *mqd, const char *msg_ptr, size_t msg_len,
k_timeout_t timeout);
Expand Down Expand Up @@ -255,9 +257,14 @@ int mq_timedsend(mqd_t mqdes, const char *msg_ptr, size_t msg_len,
unsigned int msg_prio, const struct timespec *abstime)
{
mqueue_desc *mqd = (mqueue_desc *)mqdes;
int32_t timeout = (int32_t) timespec_to_timeoutms(abstime);

return send_message(mqd, msg_ptr, msg_len, K_MSEC(timeout));
if ((abstime == NULL) || !timespec_is_valid(abstime)) {
errno = EINVAL;
return -1;
}

return send_message(mqd, msg_ptr, msg_len,
K_MSEC(timespec_to_timeoutms(CLOCK_REALTIME, abstime)));
}

/**
Expand Down Expand Up @@ -286,9 +293,14 @@ int mq_timedreceive(mqd_t mqdes, char *msg_ptr, size_t msg_len,
unsigned int *msg_prio, const struct timespec *abstime)
{
mqueue_desc *mqd = (mqueue_desc *)mqdes;
int32_t timeout = (int32_t) timespec_to_timeoutms(abstime);

return receive_message(mqd, msg_ptr, msg_len, K_MSEC(timeout));
if ((abstime == NULL) || !timespec_is_valid(abstime)) {
errno = EINVAL;
return -1;
}

return receive_message(mqd, msg_ptr, msg_len,
K_MSEC(timespec_to_timeoutms(CLOCK_REALTIME, abstime)));
}

/**
Expand Down
11 changes: 7 additions & 4 deletions lib/posix/options/mutex.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

#include "posix_clock.h"
#include "posix_internal.h"

#include <zephyr/init.h>
Expand All @@ -18,8 +19,6 @@ LOG_MODULE_REGISTER(pthread_mutex, CONFIG_PTHREAD_MUTEX_LOG_LEVEL);

static SYS_SEM_DEFINE(lock, 1, 1);

int64_t timespec_to_timeoutms(const struct timespec *abstime);

#define MUTEX_MAX_REC_LOCK 32767

/*
Expand Down Expand Up @@ -212,8 +211,12 @@ int pthread_mutex_trylock(pthread_mutex_t *m)
int pthread_mutex_timedlock(pthread_mutex_t *m,
const struct timespec *abstime)
{
int32_t timeout = (int32_t)timespec_to_timeoutms(abstime);
return acquire_mutex(m, K_MSEC(timeout));
if ((abstime == NULL) || !timespec_is_valid(abstime)) {
LOG_DBG("%s is invalid", "abstime");
return EINVAL;
}

return acquire_mutex(m, K_MSEC(timespec_to_timeoutms(CLOCK_REALTIME, abstime)));
}

/**
Expand Down
Loading