From c6607cacec7de4da89afe76154788644e1a5db89 Mon Sep 17 00:00:00 2001 From: Christopher Friedt Date: Mon, 1 Jan 2024 18:40:48 -0500 Subject: [PATCH 1/6] tests: posix: common: key: remove overspecified pthread_attr_t Much of tests/posix/common still overspecifies pthread_attr_t options. Default thread attributes are perfectly fine for the vast majority of spawned threads in this testsuite, so simply use a NULL pthread_attr_t* argument. This fixes piles of bugs because we have not properly used pthread_attr_destroy() appropriately. Signed-off-by: Christopher Friedt --- tests/posix/common/src/key.c | 171 +++++++---------------------------- 1 file changed, 35 insertions(+), 136 deletions(-) diff --git a/tests/posix/common/src/key.c b/tests/posix/common/src/key.c index 58e67a313acb..dbeddc3b6d0e 100644 --- a/tests/posix/common/src/key.c +++ b/tests/posix/common/src/key.c @@ -10,102 +10,54 @@ #include #define N_THR 2 -#define N_KEY 2 -#define STACKSZ (MAX(1024, PTHREAD_STACK_MIN) + CONFIG_TEST_EXTRA_STACK_SIZE) +#define N_KEY 2 #define BUFFSZ 48 -K_THREAD_STACK_ARRAY_DEFINE(stackp, N_THR, STACKSZ); - -pthread_key_t key, keys[N_KEY]; +static pthread_key_t key; +static pthread_key_t keys[N_KEY]; static pthread_once_t key_once = PTHREAD_ONCE_INIT; static pthread_once_t keys_once = PTHREAD_ONCE_INIT; -void *thread_top(void *p1) +static void *thread_top(void *p1) { - int ret = -1; - void *value; - void *getval; char *buffer[BUFFSZ]; value = k_malloc(sizeof(buffer)); - - zassert_true((int) POINTER_TO_INT(value), - "thread could not allocate storage"); - - ret = pthread_setspecific(key, value); - - /* TESTPOINT: Check if thread's value is associated with key */ - zassert_false(ret, "pthread_setspecific failed"); - - getval = 0; - - getval = pthread_getspecific(key); - - /* TESTPOINT: Check if pthread_getspecific returns the same value - * set by pthread_setspecific - */ - zassert_equal(value, getval, - "set and retrieved values are different"); - - printk("set value = %d and retrieved value = %d\n", - (int) POINTER_TO_INT(value), (int) POINTER_TO_INT(getval)); + zassert_not_null(value, "thread could not allocate storage"); + zassert_ok(pthread_setspecific(key, value), "pthread_setspecific failed"); + zassert_equal(pthread_getspecific(key), value, "set and retrieved values are different"); + k_free(value); return NULL; } -void *thread_func(void *p1) +static void *thread_func(void *p1) { - int i, ret = -1; - void *value; - void *getval; char *buffer[BUFFSZ]; value = k_malloc(sizeof(buffer)); - - zassert_true((int) POINTER_TO_INT(value), - "thread could not allocate storage"); - - for (i = 0; i < N_KEY; i++) { - ret = pthread_setspecific(keys[i], value); - - /* TESTPOINT: Check if thread's value is associated with keys */ - zassert_false(ret, "pthread_setspecific failed"); - } - - for (i = 0; i < N_KEY; i++) { - getval = 0; - getval = pthread_getspecific(keys[i]); - - /* TESTPOINT: Check if pthread_getspecific returns the same - * value set by pthread_setspecific for each of the keys - */ - zassert_equal(value, getval, - "set and retrieved values are different"); - - printk("key %d: set value = %d and retrieved value = %d\n", - i, (int) POINTER_TO_INT(value), - (int) POINTER_TO_INT(getval)); + zassert_not_null(value, "thread could not allocate storage"); + for (int i = 0; i < N_KEY; i++) { + zassert_ok(pthread_setspecific(keys[i], value), "pthread_setspecific failed"); + zassert_equal(pthread_getspecific(keys[i]), value, + "set and retrieved values are different"); } + k_free(value); return NULL; } static void make_key(void) { - int ret = 0; - - ret = pthread_key_create(&key, NULL); - zassert_false(ret, "insufficient memory to create key"); + zassert_ok(pthread_key_create(&key, NULL), "insufficient memory to create key"); } static void make_keys(void) { - int i, ret = 0; - - for (i = 0; i < N_KEY; i++) { - ret = pthread_key_create(&keys[i], NULL); - zassert_false(ret, "insufficient memory to create keys"); + for (int i = 0; i < N_KEY; i++) { + zassert_ok(pthread_key_create(&keys[i], NULL), + "insufficient memory to create keys"); } } @@ -124,94 +76,41 @@ static void make_keys(void) ZTEST(posix_apis, test_key_1toN_thread) { - int i, ret = -1; - - pthread_attr_t attr[N_THR]; - struct sched_param schedparam; - pthread_t newthread[N_THR]; void *retval; + pthread_t newthread[N_THR]; - ret = pthread_once(&key_once, make_key); - - /* TESTPOINT: Check if key is created */ - zassert_false(ret, "attempt to create key failed"); - - printk("\nDifferent threads set different values to same key:\n"); - - /* Creating threads with lowest application priority */ - for (i = 0; i < N_THR; i++) { - ret = pthread_attr_init(&attr[i]); - if (ret != 0) { - zassert_false(pthread_attr_destroy(&attr[i]), - "Unable to destroy pthread object attr"); - zassert_false(pthread_attr_init(&attr[i]), - "Unable to create pthread object attr"); - } - - schedparam.sched_priority = 2; - pthread_attr_setschedparam(&attr[i], &schedparam); - pthread_attr_setstack(&attr[i], &stackp[i][0], STACKSZ); + zassert_ok(pthread_once(&key_once, make_key), "attempt to create key failed"); - ret = pthread_create(&newthread[i], &attr[i], thread_top, - INT_TO_POINTER(i)); + /* Different threads set different values to same key */ - /* TESTPOINT: Check if threads are created successfully */ - zassert_false(ret, "attempt to create threads failed"); + for (int i = 0; i < N_THR; i++) { + zassert_ok(pthread_create(&newthread[i], NULL, thread_top, NULL), + "attempt to create thread %d failed", i); } - for (i = 0; i < N_THR; i++) { - printk("thread %d: ", i); - pthread_join(newthread[i], &retval); + for (int i = 0; i < N_THR; i++) { + zassert_ok(pthread_join(newthread[i], &retval), "failed to join thread %d", i); } - ret = pthread_key_delete(key); - - /* TESTPOINT: Check if key is deleted */ - zassert_false(ret, "attempt to delete key failed"); - printk("\n"); + zassert_ok(pthread_key_delete(key), "attempt to delete key failed"); } ZTEST(posix_apis, test_key_Nto1_thread) { - int i, ret = -1; - - pthread_attr_t attr; - struct sched_param schedparam; pthread_t newthread; - ret = pthread_once(&keys_once, make_keys); - - /* TESTPOINT: Check if keys are created successfully */ - zassert_false(ret, "attempt to create keys failed"); - - printk("\nSingle thread associates its value with different keys:\n"); - ret = pthread_attr_init(&attr); - if (ret != 0) { - zassert_false(pthread_attr_destroy(&attr), - "Unable to destroy pthread object attr"); - zassert_false(pthread_attr_init(&attr), - "Unable to create pthread object attr"); - } - - schedparam.sched_priority = 2; - pthread_attr_setschedparam(&attr, &schedparam); - pthread_attr_setstack(&attr, &stackp[0][0], STACKSZ); - - ret = pthread_create(&newthread, &attr, thread_func, - (void *)0); + zassert_ok(pthread_once(&keys_once, make_keys), "attempt to create keys failed"); - /*TESTPOINT: Check if thread is created successfully */ - zassert_false(ret, "attempt to create thread failed"); + /* Single thread associates its value with different keys */ - pthread_join(newthread, NULL); + zassert_ok(pthread_create(&newthread, NULL, thread_func, NULL), + "attempt to create thread failed"); - for (i = 0; i < N_KEY; i++) { - ret = pthread_key_delete(keys[i]); + zassert_ok(pthread_join(newthread, NULL), "failed to join thread"); - /* TESTPOINT: Check if keys are deleted */ - zassert_false(ret, "attempt to delete keys failed"); + for (int i = 0; i < N_KEY; i++) { + zassert_ok(pthread_key_delete(keys[i]), "attempt to delete keys[%d] failed", i); } - printk("\n"); } ZTEST(posix_apis, test_key_resource_leak) From 0e2d2aca3244ec650413065099e7ce34c8ad76e2 Mon Sep 17 00:00:00 2001 From: Christopher Friedt Date: Mon, 1 Jan 2024 18:44:32 -0500 Subject: [PATCH 2/6] tests: posix: common: mqueue: remove overspecified pthread_attr_t Much of tests/posix/common still overspecifies pthread_attr_t options. Default thread attributes are perfectly fine for the vast majority of spawned threads in this testsuite, so simply use a NULL pthread_attr_t* argument. This fixes piles of bugs because we have not properly used pthread_attr_destroy() appropriately. Signed-off-by: Christopher Friedt --- tests/posix/common/src/mqueue.c | 33 +++++++++------------------------ 1 file changed, 9 insertions(+), 24 deletions(-) diff --git a/tests/posix/common/src/mqueue.c b/tests/posix/common/src/mqueue.c index 65df6fbe7772..4ddfdea2da9a 100644 --- a/tests/posix/common/src/mqueue.c +++ b/tests/posix/common/src/mqueue.c @@ -11,15 +11,12 @@ #include #include -#define N_THR 2 -#define STACKSZ (MAX(1024, PTHREAD_STACK_MIN) + CONFIG_TEST_EXTRA_STACK_SIZE) +#define N_THR 2 #define SENDER_THREAD 0 #define RECEIVER_THREAD 1 #define MESSAGE_SIZE 16 #define MESG_COUNT_PERMQ 4 -K_THREAD_STACK_ARRAY_DEFINE(stacks, N_THR, STACKSZ); - char queue[16] = "server"; char send_data[MESSAGE_SIZE] = "timed data send"; @@ -60,7 +57,8 @@ void *receiver_thread(void *p1) clock_gettime(CLOCK_MONOTONIC, &curtime); curtime.tv_sec += 1; mq_timedreceive(mqd, rec_data, MESSAGE_SIZE, 0, &curtime); - zassert_false(strcmp(rec_data, send_data), "Error in data reception"); + zassert_false(strcmp(rec_data, send_data), "Error in data reception. exp: %s act: %s", + send_data, rec_data); usleep(USEC_PER_MSEC); zassert_false(mq_close(mqd), "unable to close message queue descriptor."); @@ -72,9 +70,9 @@ ZTEST(posix_apis, test_mqueue) { mqd_t mqd; struct mq_attr attrs; - int32_t mode = 0777, flags = O_RDWR | O_CREAT, ret, i; + int32_t mode = 0777; + int flags = O_RDWR | O_CREAT; void *retval; - pthread_attr_t attr[N_THR]; pthread_t newthread[N_THR]; attrs.mq_msgsize = MESSAGE_SIZE; @@ -82,28 +80,15 @@ ZTEST(posix_apis, test_mqueue) mqd = mq_open(queue, flags, mode, &attrs); - for (i = 0; i < N_THR; i++) { + for (int i = 0; i < N_THR; i++) { /* Creating threads */ - zassert_ok(pthread_attr_init(&attr[i])); - pthread_attr_setstack(&attr[i], &stacks[i][0], STACKSZ); - - if (i % 2) { - ret = pthread_create(&newthread[i], &attr[i], - sender_thread, - INT_TO_POINTER(i)); - } else { - ret = pthread_create(&newthread[i], &attr[i], - receiver_thread, - INT_TO_POINTER(i)); - } - - zassert_false(ret, "Not enough space to create new thread"); - zassert_equal(pthread_attr_destroy(&attr[i]), 0); + zassert_ok(pthread_create(&newthread[i], NULL, + (i % 2 == 0) ? receiver_thread : sender_thread, NULL)); } usleep(USEC_PER_MSEC * 10U); - for (i = 0; i < N_THR; i++) { + for (int i = 0; i < N_THR; i++) { pthread_join(newthread[i], &retval); } From 9e421ac96c90b0fc7f6c139b3e2ae7165f01a390 Mon Sep 17 00:00:00 2001 From: Christopher Friedt Date: Mon, 1 Jan 2024 18:45:38 -0500 Subject: [PATCH 3/6] tests: posix: common: mutex: remove overspecified pthread_attr_t Much of tests/posix/common still overspecifies pthread_attr_t options. Default thread attributes are perfectly fine for the vast majority of spawned threads in this testsuite, so simply use a NULL pthread_attr_t* argument. This fixes piles of bugs because we have not properly used pthread_attr_destroy() appropriately. Signed-off-by: Christopher Friedt --- tests/posix/common/src/mutex.c | 139 +++++++++------------------------ 1 file changed, 39 insertions(+), 100 deletions(-) diff --git a/tests/posix/common/src/mutex.c b/tests/posix/common/src/mutex.c index 6a63f98e4510..ded1055f7dce 100644 --- a/tests/posix/common/src/mutex.c +++ b/tests/posix/common/src/mutex.c @@ -10,14 +10,8 @@ #include #include -#define STACK_SIZE (MAX(1024, PTHREAD_STACK_MIN) + CONFIG_TEST_EXTRA_STACK_SIZE) - -static K_THREAD_STACK_DEFINE(stack, STACK_SIZE); - #define SLEEP_MS 100 -pthread_mutex_t mutex1; -pthread_mutex_t mutex2; pthread_mutex_t mutex; void *normal_mutex_entry(void *p1) @@ -27,7 +21,7 @@ void *normal_mutex_entry(void *p1) /* Sleep for maximum 300 ms as main thread is sleeping for 100 ms */ for (i = 0; i < 3; i++) { - rc = pthread_mutex_trylock(&mutex1); + rc = pthread_mutex_trylock(&mutex); if (rc == 0) { break; } @@ -36,74 +30,62 @@ void *normal_mutex_entry(void *p1) zassert_false(rc, "try lock failed"); TC_PRINT("mutex lock is taken\n"); - zassert_false(pthread_mutex_unlock(&mutex1), "mutex unlock is failed"); + zassert_false(pthread_mutex_unlock(&mutex), "mutex unlock is failed"); return NULL; } void *recursive_mutex_entry(void *p1) { - zassert_false(pthread_mutex_lock(&mutex2), "mutex is not taken"); - zassert_false(pthread_mutex_lock(&mutex2), "mutex is not taken 2nd time"); + zassert_false(pthread_mutex_lock(&mutex), "mutex is not taken"); + zassert_false(pthread_mutex_lock(&mutex), "mutex is not taken 2nd time"); TC_PRINT("recursive mutex lock is taken\n"); - zassert_false(pthread_mutex_unlock(&mutex2), "mutex is not unlocked"); - zassert_false(pthread_mutex_unlock(&mutex2), "mutex is not unlocked"); + zassert_false(pthread_mutex_unlock(&mutex), "mutex is not unlocked"); + zassert_false(pthread_mutex_unlock(&mutex), "mutex is not unlocked"); return NULL; } -/** - * @brief Test to demonstrate PTHREAD_MUTEX_NORMAL - * - * @details Mutex type is setup as normal. pthread_mutex_trylock - * and pthread_mutex_lock are tested with mutex type being - * normal. - */ -ZTEST(posix_apis, test_mutex_normal) +static void test_mutex_common(int type, void *(*entry)(void *arg)) { - pthread_t thread_1; - pthread_attr_t attr; - pthread_mutexattr_t mut_attr; + pthread_t th; + int protocol; + int actual_type; struct sched_param schedparam; - int schedpolicy = SCHED_FIFO; - int ret, type, protocol, temp; + pthread_mutexattr_t mut_attr = {0}; schedparam.sched_priority = 2; - ret = pthread_attr_init(&attr); - if (ret != 0) { - zassert_false(pthread_attr_destroy(&attr), - "Unable to destroy pthread object attrib"); - zassert_false(pthread_attr_init(&attr), "Unable to create pthread object attrib"); - } - - pthread_attr_setstack(&attr, &stack, STACK_SIZE); - pthread_attr_setschedpolicy(&attr, schedpolicy); - pthread_attr_setschedparam(&attr, &schedparam); - temp = pthread_mutexattr_settype(&mut_attr, PTHREAD_MUTEX_NORMAL); - zassert_false(temp, "setting mutex type is failed"); - temp = pthread_mutex_init(&mutex1, &mut_attr); - zassert_false(temp, "mutex initialization is failed"); + zassert_ok(pthread_mutexattr_settype(&mut_attr, type), "setting mutex type is failed"); + zassert_ok(pthread_mutex_init(&mutex, &mut_attr), "mutex initialization is failed"); - temp = pthread_mutexattr_gettype(&mut_attr, &type); - zassert_false(temp, "reading mutex type is failed"); - temp = pthread_mutexattr_getprotocol(&mut_attr, &protocol); - zassert_false(temp, "reading mutex protocol is failed"); + zassert_ok(pthread_mutexattr_gettype(&mut_attr, &actual_type), + "reading mutex type is failed"); + zassert_ok(pthread_mutexattr_getprotocol(&mut_attr, &protocol), + "reading mutex protocol is failed"); - pthread_mutex_lock(&mutex1); - - zassert_equal(type, PTHREAD_MUTEX_NORMAL, "mutex type is not normal"); + zassert_ok(pthread_mutex_lock(&mutex)); + zassert_equal(actual_type, type, "mutex type is not normal"); zassert_equal(protocol, PTHREAD_PRIO_NONE, "mutex protocol is not prio_none"); - ret = pthread_create(&thread_1, &attr, &normal_mutex_entry, NULL); - if (ret) { - TC_PRINT("Thread1 creation failed %d", ret); - } + zassert_ok(pthread_create(&th, NULL, entry, NULL)); + k_msleep(SLEEP_MS); - pthread_mutex_unlock(&mutex1); + zassert_ok(pthread_mutex_unlock(&mutex)); + + zassert_ok(pthread_join(th, NULL)); + zassert_ok(pthread_mutex_destroy(&mutex), "Destroying mutex is failed"); +} - pthread_join(thread_1, NULL); - temp = pthread_mutex_destroy(&mutex1); - zassert_false(temp, "Destroying mutex is failed"); +/** + * @brief Test to demonstrate PTHREAD_MUTEX_NORMAL + * + * @details Mutex type is setup as normal. pthread_mutex_trylock + * and pthread_mutex_lock are tested with mutex type being + * normal. + */ +ZTEST(posix_apis, test_mutex_normal) +{ + test_mutex_common(PTHREAD_MUTEX_NORMAL, normal_mutex_entry); } /** @@ -115,45 +97,7 @@ ZTEST(posix_apis, test_mutex_normal) */ ZTEST(posix_apis, test_mutex_recursive) { - pthread_t thread_2; - pthread_attr_t attr2; - pthread_mutexattr_t mut_attr2; - struct sched_param schedparam2; - int schedpolicy = SCHED_FIFO; - int ret, type, protocol, temp; - - schedparam2.sched_priority = 2; - ret = pthread_attr_init(&attr2); - if (ret != 0) { - zassert_false(pthread_attr_destroy(&attr2), - "Unable to destroy pthread object attrib"); - zassert_false(pthread_attr_init(&attr2), "Unable to create pthread object attrib"); - } - - pthread_attr_setstack(&attr2, &stack, STACK_SIZE); - pthread_attr_setschedpolicy(&attr2, schedpolicy); - pthread_attr_setschedparam(&attr2, &schedparam2); - - temp = pthread_mutexattr_settype(&mut_attr2, PTHREAD_MUTEX_RECURSIVE); - zassert_false(temp, "setting mutex2 type is failed"); - temp = pthread_mutex_init(&mutex2, &mut_attr2); - zassert_false(temp, "mutex2 initialization is failed"); - - temp = pthread_mutexattr_gettype(&mut_attr2, &type); - zassert_false(temp, "reading mutex2 type is failed"); - temp = pthread_mutexattr_getprotocol(&mut_attr2, &protocol); - zassert_false(temp, "reading mutex2 protocol is failed"); - - zassert_equal(type, PTHREAD_MUTEX_RECURSIVE, "mutex2 type is not recursive"); - - zassert_equal(protocol, PTHREAD_PRIO_NONE, "mutex2 protocol is not prio_none"); - ret = pthread_create(&thread_2, &attr2, &recursive_mutex_entry, NULL); - - zassert_false(ret, "Thread2 creation failed"); - - pthread_join(thread_2, NULL); - temp = pthread_mutex_destroy(&mutex2); - zassert_false(temp, "Destroying mutex2 is failed"); + test_mutex_common(PTHREAD_MUTEX_RECURSIVE, recursive_mutex_entry); } /** @@ -228,23 +172,19 @@ ZTEST(posix_apis, test_mutex_timedlock) { void *ret; pthread_t th; - pthread_attr_t attr; - - zassert_ok(pthread_attr_init(&attr)); - zassert_ok(pthread_attr_setstack(&attr, &stack, STACK_SIZE)); zassert_ok(pthread_mutex_init(&mutex, NULL)); printk("Expecting timedlock with timeout of %d ms to fail\n", TIMEDLOCK_TIMEOUT_MS); zassert_ok(pthread_mutex_lock(&mutex)); - zassert_ok(pthread_create(&th, &attr, test_mutex_timedlock_fn, &mutex)); + zassert_ok(pthread_create(&th, NULL, test_mutex_timedlock_fn, &mutex)); zassert_ok(pthread_join(th, &ret)); /* ensure timeout occurs */ zassert_equal(ETIMEDOUT, POINTER_TO_INT(ret)); printk("Expecting timedlock with timeout of %d ms to succeed after 100ms\n", TIMEDLOCK_TIMEOUT_MS); - zassert_ok(pthread_create(&th, &attr, test_mutex_timedlock_fn, &mutex)); + zassert_ok(pthread_create(&th, NULL, test_mutex_timedlock_fn, &mutex)); /* unlock before timeout expires */ k_msleep(TIMEDLOCK_TIMEOUT_DELAY_MS); zassert_ok(pthread_mutex_unlock(&mutex)); @@ -253,5 +193,4 @@ ZTEST(posix_apis, test_mutex_timedlock) zassert_ok(POINTER_TO_INT(ret)); zassert_ok(pthread_mutex_destroy(&mutex)); - zassert_ok(pthread_attr_destroy(&attr)); } From 4027a39d5284ff649515f575e64526e7bc161a45 Mon Sep 17 00:00:00 2001 From: Christopher Friedt Date: Mon, 1 Jan 2024 19:20:09 -0500 Subject: [PATCH 4/6] tests: posix: common: pthread: do not overspecify pthread_attr_t Much of tests/posix/common still overspecifies pthread_attr_t options. Default thread attributes are perfectly fine for the vast majority of spawned threads in this testsuite, so simply use a NULL pthread_attr_t* argument. This fixes piles of bugs because we have not properly used pthread_attr_destroy() appropriately. This is only a partial cleanup for pthread.c Signed-off-by: Christopher Friedt --- tests/posix/common/src/pthread.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/tests/posix/common/src/pthread.c b/tests/posix/common/src/pthread.c index 16c4af2e3602..7cf2405c2b44 100644 --- a/tests/posix/common/src/pthread.c +++ b/tests/posix/common/src/pthread.c @@ -710,7 +710,7 @@ ZTEST(posix_apis, test_sched_policy) } /* get pmin and pmax for policies[policy] */ - for (int i = 0; i < 2; ++i) { + for (int i = 0; i < ARRAY_SIZE(prios); ++i) { errno = 0; if (i == 0) { pmin = sched_get_priority_min(policies[policy]); @@ -749,7 +749,7 @@ ZTEST(posix_apis, test_sched_policy) zassert_equal(pmax, nprio[policy] - 1, "unexpected pmax for %s", policy_names[policy]); /* test happy paths */ - for (int i = 0; i < 2; ++i) { + for (int i = 0; i < ARRAY_SIZE(prios); ++i) { /* create threads with min and max priority levels */ zassert_ok(pthread_attr_init(&attr), "pthread_attr_init() failed for %s (%d) of %s", prios[i], @@ -774,6 +774,10 @@ ZTEST(posix_apis, test_sched_policy) zassert_ok(pthread_join(th, NULL), "pthread_join() failed for %s (%d) of %s", prios[i], param.sched_priority, policy_names[policy]); + + zassert_ok(pthread_attr_destroy(&attr), + "pthread_attr_destroy() failed for %s (%d) of %s", prios[i], + param.sched_priority, policy_names[policy]); } } } @@ -845,12 +849,8 @@ ZTEST(posix_apis, test_pthread_return_val) { pthread_t pth; void *ret = NULL; - pthread_attr_t attr; - - zassert_ok(pthread_attr_init(&attr)); - zassert_ok(pthread_attr_setstack(&attr, &stack_e[0][0], STACKS)); - zassert_ok(pthread_create(&pth, &attr, non_null_retval, NULL)); + zassert_ok(pthread_create(&pth, NULL, non_null_retval, NULL)); zassert_ok(pthread_join(pth, &ret)); zassert_equal(ret, (void *)BIOS_FOOD); } @@ -865,12 +865,8 @@ static void *detached(void *arg) ZTEST(posix_apis, test_pthread_join_detached) { pthread_t pth; - pthread_attr_t attr; - - zassert_ok(pthread_attr_init(&attr)); - zassert_ok(pthread_attr_setstack(&attr, &stack_e[0][0], STACKS)); - zassert_ok(pthread_create(&pth, &attr, detached, NULL)); + zassert_ok(pthread_create(&pth, NULL, detached, NULL)); zassert_ok(pthread_detach(pth)); /* note, this was required to be EINVAL previously but is now undefined behaviour */ zassert_not_equal(0, pthread_join(pth, NULL)); From c629ef322ed73a7eecd4cefc25bcf44ff2485218 Mon Sep 17 00:00:00 2001 From: Christopher Friedt Date: Tue, 2 Jan 2024 06:58:14 -0500 Subject: [PATCH 5/6] tests: posix: common: sem: remove overspecified pthread_attr_t Much of tests/posix/common still overspecifies pthread_attr_t options. Default thread attributes are perfectly fine for the vast majority of spawned threads in this testsuite, so simply use a NULL pthread_attr_t* argument. This fixes piles of bugs because we have not properly used pthread_attr_destroy() appropriately. Signed-off-by: Christopher Friedt --- tests/posix/common/src/semaphore.c | 42 +++--------------------------- 1 file changed, 4 insertions(+), 38 deletions(-) diff --git a/tests/posix/common/src/semaphore.c b/tests/posix/common/src/semaphore.c index fef4ddfb0fe9..921ddbb4a416 100644 --- a/tests/posix/common/src/semaphore.c +++ b/tests/posix/common/src/semaphore.c @@ -11,15 +11,8 @@ #include #include -#define STACK_SIZE (MAX(1024, PTHREAD_STACK_MIN) + CONFIG_TEST_EXTRA_STACK_SIZE) - -sem_t sema; -void *dummy_sem; - -struct sched_param schedparam; -int schedpolicy = SCHED_FIFO; - -static K_THREAD_STACK_DEFINE(stack, STACK_SIZE); +static sem_t sema; +static void *dummy_sem; static void *child_func(void *p1) { @@ -27,34 +20,12 @@ static void *child_func(void *p1) return NULL; } -void initialize_thread_attr(pthread_attr_t *attr) -{ - int ret; - - schedparam.sched_priority = 1; - - ret = pthread_attr_init(attr); - if (ret != 0) { - zassert_equal(pthread_attr_destroy(attr), 0, - "Unable to destroy pthread object attrib"); - zassert_equal(pthread_attr_init(attr), 0, - "Unable to create pthread object attrib"); - } - - pthread_attr_setstack(attr, &stack, STACK_SIZE); - pthread_attr_setschedpolicy(attr, schedpolicy); - pthread_attr_setschedparam(attr, &schedparam); -} - ZTEST(posix_apis, test_semaphore) { pthread_t thread1, thread2; - pthread_attr_t attr1, attr2; int val, ret; struct timespec abstime; - initialize_thread_attr(&attr1); - /* TESTPOINT: Check if sema value is less than * CONFIG_SEM_VALUE_MAX */ @@ -79,7 +50,7 @@ ZTEST(posix_apis, test_semaphore) zassert_equal(sem_trywait(&sema), -1); zassert_equal(errno, EAGAIN); - ret = pthread_create(&thread1, &attr1, child_func, NULL); + ret = pthread_create(&thread1, NULL, child_func, NULL); zassert_equal(ret, 0, "Thread creation failed"); zassert_equal(clock_gettime(CLOCK_REALTIME, &abstime), 0, @@ -105,9 +76,6 @@ ZTEST(posix_apis, test_semaphore) zassert_equal(sem_destroy(&sema), 0, "semaphore is not destroyed"); - zassert_equal(pthread_attr_destroy(&attr1), 0, - "Unable to destroy pthread object attrib"); - /* TESTPOINT: Initialize sema with 1 */ zassert_equal(sem_init(&sema, 0, 1), 0, "sem_init failed"); zassert_equal(sem_getvalue(&sema, &val), 0); @@ -120,9 +88,7 @@ ZTEST(posix_apis, test_semaphore) /* TESTPOINT: take semaphore which is initialized with 1 */ zassert_equal(sem_trywait(&sema), 0); - initialize_thread_attr(&attr2); - - zassert_equal(pthread_create(&thread2, &attr2, child_func, NULL), 0, + zassert_equal(pthread_create(&thread2, NULL, child_func, NULL), 0, "Thread creation failed"); /* TESTPOINT: Wait and acquire semaphore till thread2 gives */ From 80d9379a00bc4600a35bd37fd8e07b871a3d1038 Mon Sep 17 00:00:00 2001 From: Christopher Friedt Date: Tue, 2 Jan 2024 12:25:59 -0500 Subject: [PATCH 6/6] tests: posix: common: rwlock: remove overspecified pthread_attr_t Much of tests/posix/common still overspecifies pthread_attr_t options. Default thread attributes are perfectly fine for the vast majority of spawned threads in this testsuite, so simply use a NULL pthread_attr_t* argument. This fixes piles of bugs because we have not properly used pthread_attr_destroy() appropriately. Signed-off-by: Christopher Friedt --- tests/posix/common/src/rwlock.c | 114 ++++++++++++-------------------- 1 file changed, 43 insertions(+), 71 deletions(-) diff --git a/tests/posix/common/src/rwlock.c b/tests/posix/common/src/rwlock.c index 43c573a70075..b7d3ac2abe21 100644 --- a/tests/posix/common/src/rwlock.c +++ b/tests/posix/common/src/rwlock.c @@ -6,59 +6,50 @@ #include +#include #include #include #define N_THR 3 -#define STACKSZ (MAX(1024, PTHREAD_STACK_MIN) + CONFIG_TEST_EXTRA_STACK_SIZE) -K_THREAD_STACK_ARRAY_DEFINE(stack, N_THR, STACKSZ); -pthread_rwlock_t rwlock; +LOG_MODULE_REGISTER(posix_rwlock_test); + +static pthread_rwlock_t rwlock; static void *thread_top(void *p1) { - pthread_t pthread; - uint32_t policy, ret = 0U; - struct sched_param param; - int id = POINTER_TO_INT(p1); - - pthread = (pthread_t) pthread_self(); - pthread_getschedparam(pthread, &policy, ¶m); - printk("Thread %d scheduling policy = %d & priority %d started\n", - id, policy, param.sched_priority); + int ret; + pthread_t id; + id = (pthread_t)pthread_self(); ret = pthread_rwlock_tryrdlock(&rwlock); - if (ret) { - printk("Not able to get RD lock on trying, try again\n"); - zassert_false(pthread_rwlock_rdlock(&rwlock), - "Failed to acquire write lock"); + if (ret != 0) { + LOG_DBG("Not able to get RD lock on trying, try again"); + zassert_ok(pthread_rwlock_rdlock(&rwlock), "Failed to acquire write lock"); } - printk("Thread %d got RD lock\n", id); + LOG_DBG("Thread %d got RD lock", id); usleep(USEC_PER_MSEC); - printk("Thread %d releasing RD lock\n", id); - zassert_false(pthread_rwlock_unlock(&rwlock), "Failed to unlock"); + LOG_DBG("Thread %d releasing RD lock", id); + zassert_ok(pthread_rwlock_unlock(&rwlock), "Failed to unlock"); - printk("Thread %d acquiring WR lock\n", id); + LOG_DBG("Thread %d acquiring WR lock", id); ret = pthread_rwlock_trywrlock(&rwlock); - if (ret != 0U) { - zassert_false(pthread_rwlock_wrlock(&rwlock), - "Failed to acquire WR lock"); + if (ret != 0) { + zassert_ok(pthread_rwlock_wrlock(&rwlock), "Failed to acquire WR lock"); } - printk("Thread %d acquired WR lock\n", id); + LOG_DBG("Thread %d acquired WR lock", id); usleep(USEC_PER_MSEC); - printk("Thread %d releasing WR lock\n", id); - zassert_false(pthread_rwlock_unlock(&rwlock), "Failed to unlock"); - pthread_exit(NULL); + LOG_DBG("Thread %d releasing WR lock", id); + zassert_ok(pthread_rwlock_unlock(&rwlock), "Failed to unlock"); + return NULL; } ZTEST(posix_apis, test_rw_lock) { - int32_t i, ret; - pthread_attr_t attr[N_THR]; - struct sched_param schedparam; + int ret; pthread_t newthread[N_THR]; struct timespec time; void *status; @@ -75,72 +66,53 @@ ZTEST(posix_apis, test_rw_lock) zassert_equal(pthread_rwlock_timedrdlock(&rwlock, &time), EINVAL); zassert_equal(pthread_rwlock_unlock(&rwlock), EINVAL); - zassert_false(pthread_rwlock_init(&rwlock, NULL), - "Failed to create rwlock"); - printk("\nmain acquire WR lock and 3 threads acquire RD lock\n"); - zassert_false(pthread_rwlock_timedwrlock(&rwlock, &time), - "Failed to acquire write lock"); + zassert_ok(pthread_rwlock_init(&rwlock, NULL), "Failed to create rwlock"); + LOG_DBG("main acquire WR lock and 3 threads acquire RD lock"); + zassert_ok(pthread_rwlock_timedwrlock(&rwlock, &time), "Failed to acquire write lock"); /* Creating N preemptive threads in increasing order of priority */ - for (i = 0; i < N_THR; i++) { - zassert_equal(pthread_attr_init(&attr[i]), 0, - "Unable to create pthread object attrib"); - - /* Setting scheduling priority */ - schedparam.sched_priority = i + 1; - pthread_attr_setschedparam(&attr[i], &schedparam); - - /* Setting stack */ - pthread_attr_setstack(&attr[i], &stack[i][0], STACKSZ); - - ret = pthread_create(&newthread[i], &attr[i], thread_top, - INT_TO_POINTER(i)); - zassert_false(ret, "Low memory to thread new thread"); - + for (int i = 0; i < N_THR; i++) { + zassert_ok(pthread_create(&newthread[i], NULL, thread_top, NULL), + "Low memory to thread new thread"); } /* Delay to give change to child threads to run */ usleep(USEC_PER_MSEC); - printk("Parent thread releasing WR lock\n"); - zassert_false(pthread_rwlock_unlock(&rwlock), "Failed to unlock"); + LOG_DBG("Parent thread releasing WR lock"); + zassert_ok(pthread_rwlock_unlock(&rwlock), "Failed to unlock"); /* Let child threads acquire RD Lock */ usleep(USEC_PER_MSEC); - printk("Parent thread acquiring WR lock again\n"); + LOG_DBG("Parent thread acquiring WR lock again"); time.tv_sec = 2; time.tv_nsec = 0; ret = pthread_rwlock_timedwrlock(&rwlock, &time); - if (ret) { - zassert_false(pthread_rwlock_wrlock(&rwlock), - "Failed to acquire write lock"); + zassert_ok(pthread_rwlock_wrlock(&rwlock), "Failed to acquire write lock"); } - printk("Parent thread acquired WR lock again\n"); + LOG_DBG("Parent thread acquired WR lock again"); usleep(USEC_PER_MSEC); - printk("Parent thread releasing WR lock again\n"); - zassert_false(pthread_rwlock_unlock(&rwlock), "Failed to unlock"); + LOG_DBG("Parent thread releasing WR lock again"); + zassert_ok(pthread_rwlock_unlock(&rwlock), "Failed to unlock"); - printk("\n3 threads acquire WR lock\n"); - printk("Main thread acquiring RD lock\n"); + LOG_DBG("3 threads acquire WR lock"); + LOG_DBG("Main thread acquiring RD lock"); ret = pthread_rwlock_timedrdlock(&rwlock, &time); - if (ret != 0) { - zassert_false(pthread_rwlock_rdlock(&rwlock), "Failed to lock"); + zassert_ok(pthread_rwlock_rdlock(&rwlock), "Failed to lock"); } - printk("Main thread acquired RD lock\n"); + LOG_DBG("Main thread acquired RD lock"); usleep(USEC_PER_MSEC); - printk("Main thread releasing RD lock\n"); - zassert_false(pthread_rwlock_unlock(&rwlock), "Failed to unlock"); + LOG_DBG("Main thread releasing RD lock"); + zassert_ok(pthread_rwlock_unlock(&rwlock), "Failed to unlock"); - for (i = 0; i < N_THR; i++) { - zassert_false(pthread_join(newthread[i], &status), - "Failed to join"); + for (int i = 0; i < N_THR; i++) { + zassert_ok(pthread_join(newthread[i], &status), "Failed to join"); } - zassert_false(pthread_rwlock_destroy(&rwlock), - "Failed to destroy rwlock"); + zassert_ok(pthread_rwlock_destroy(&rwlock), "Failed to destroy rwlock"); }