[PATCH 1/1] Implementation / tests for pthread_cond_clockwait()
Joel Sherrill
joel at rtems.org
Wed Aug 18 18:44:23 UTC 2021
On Wed, Aug 18, 2021 at 6:17 AM Matt Joyce <mfjoyce2004 at gmail.com> wrote:
>
> Added implementation of the pthread_cond_clockwait()
> method to cpukit/posix/src/condclockwait.c. Additional
> logic added to condwaitsupp.c to implement new method.
> pthread_cond_clockwait() has been added to the Issue 8
> POSIX Standard.
>
> psx17 test added to testsuites/psxtests to test the
> newly added method.
> ---
> cpukit/include/rtems/posix/condimpl.h | 1 +
> cpukit/posix/src/condclockwait.c | 69 +++
> cpukit/posix/src/condtimedwait.c | 1 +
> cpukit/posix/src/condwait.c | 1 +
> cpukit/posix/src/condwaitsupp.c | 64 ++-
> spec/build/testsuites/psxtests/grp.yml | 2 +
> spec/build/testsuites/psxtests/psx17.yml | 20 +
> testsuites/psxtests/Makefile.am | 8 +
> testsuites/psxtests/configure.ac | 1 +
> testsuites/psxtests/psx17/init.c | 425 ++++++++++++++++++
> testsuites/psxtests/psx17/psx17.doc | 45 ++
> testsuites/psxtests/psx17/psx17.scn | 82 ++++
> testsuites/psxtests/psx17/system.h | 57 +++
> .../psxhdrs/pthread/pthread_cond_clockwait.c | 2 +-
> 14 files changed, 761 insertions(+), 17 deletions(-)
> create mode 100644 cpukit/posix/src/condclockwait.c
> create mode 100644 spec/build/testsuites/psxtests/psx17.yml
> create mode 100644 testsuites/psxtests/psx17/init.c
> create mode 100644 testsuites/psxtests/psx17/psx17.doc
> create mode 100644 testsuites/psxtests/psx17/psx17.scn
> create mode 100644 testsuites/psxtests/psx17/system.h
>
> diff --git a/cpukit/include/rtems/posix/condimpl.h b/cpukit/include/rtems/posix/condimpl.h
> index 66e09bf6d8..6efbc3af70 100644
> --- a/cpukit/include/rtems/posix/condimpl.h
> +++ b/cpukit/include/rtems/posix/condimpl.h
> @@ -152,6 +152,7 @@ int _POSIX_Condition_variables_Signal_support(
> int _POSIX_Condition_variables_Wait_support(
> pthread_cond_t *cond,
> pthread_mutex_t *mutex,
> + clockid_t clock_id,
> const struct timespec *abstime
The clock_id parameter should be aligned with the rest.
> );
>
> diff --git a/cpukit/posix/src/condclockwait.c b/cpukit/posix/src/condclockwait.c
> new file mode 100644
> index 0000000000..4104235706
> --- /dev/null
> +++ b/cpukit/posix/src/condclockwait.c
> @@ -0,0 +1,69 @@
> +/**
> + * @file
> + *
> + * @ingroup POSIXAPI
> + *
> + * @brief Waiting on a Condition
> + */
> +
> +/*
> +* Copyright (C) 2021 Matthew Joyce
General question. Do we have a master list Matthew needs to be added to?
Or is that just documentation?
> +*
> +* Redistribution and use in source and binary forms, with or without
> +* modification, are permitted provided that the following conditions
> +* are met:
> +* 1. Redistributions of source code must retain the above copyright
> +* notice, this list of conditions and the following disclaimer.
> +* 2. Redistributions in binary form must reproduce the above copyright
> +* notice, this list of conditions and the following disclaimer in the
> +* documentation and/or other materials provided with the distribution.
> +*
> +* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
> +* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> +* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> +* ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
> +* LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> +* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> +* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> +* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> +* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> +* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> +* POSSIBILITY OF SUCH DAMAGE.
> +*/
> +
> +/* Defining to have access to function prototype in libc/include/pthread.h */
> +#define _GNU_SOURCE
> +
> +#ifdef HAVE_CONFIG_H
> +#include "config.h"
> +#endif
> +
> +#include <rtems/posix/condimpl.h>
> +#include <rtems/score/todimpl.h>
> +
> +/*
> + * pthread_cond_clockwait() appears in the Issue 8 POSIX Standard
> + */
> +
> +int pthread_cond_clockwait(
> + pthread_cond_t *restrict cond,
> + pthread_mutex_t *restrict mutex,
> + clockid_t clock_id,
Indentation again.
> + const struct timespec *restrict abstime
> +)
> +{
> + if ( abstime == NULL ) {
> + return EINVAL; /* not specified */
> + }
I checked Issue 8 (draft) and NULL is not specified. Please add that as
a comment above the if. More like "POSIX Issue 8 does not specify that
EINVAL is returned when abstime equals NULL"
I think it is implied that we are protecting ourselves from a NULL access.
FWIW glibc and cygwin tend to avoid NULL checks and just let the process
fail with an address access exception. We don't have that luxury to be lazy
and avoid the check.
> +
> + if ( clock_id != CLOCK_REALTIME && clock_id != CLOCK_MONOTONIC ) {
> + return EINVAL; /* not specified */
> + }
This is specified at line 51714 in the PDF. Comment is wrong.
Please rewrite so coverage testing and MCDC analysis is easier. Use simple
expressions with a single condition when possible. This should be equivalent in
this case:
if (clock_id != CLOCK_REALTIME) {
if (clock_id != CLOCK_MONOTONIC) {
return EINVALl
}
}
MCDC analysis is used for safety critical code and requires you to exercise
a sufficient set of true/false values to exercise compound expressions. If you
can write simple single condition expressions, then there is a logical operation
followed by a conditional branch. If you know the branch was both taken and
not taken, then you are done. In this case, a test case with CLOCK_REALTIME,
one with CLOCK_MONOTONIC, and one bad clock_id covers this.
> +
> + return _POSIX_Condition_variables_Wait_support(
> + cond,
> + mutex,
> + clock_id,
> + abstime
> + );
> +}
> diff --git a/cpukit/posix/src/condtimedwait.c b/cpukit/posix/src/condtimedwait.c
> index 0bc8bfc18e..53fd88efc6 100644
> --- a/cpukit/posix/src/condtimedwait.c
> +++ b/cpukit/posix/src/condtimedwait.c
> @@ -38,6 +38,7 @@ int pthread_cond_timedwait(
> return _POSIX_Condition_variables_Wait_support(
> cond,
> mutex,
> + NULL,
> abstime
> );
> }
> diff --git a/cpukit/posix/src/condwait.c b/cpukit/posix/src/condwait.c
> index 09431e216d..6e9ca4854f 100644
> --- a/cpukit/posix/src/condwait.c
> +++ b/cpukit/posix/src/condwait.c
> @@ -54,6 +54,7 @@ int pthread_cond_wait(
> return _POSIX_Condition_variables_Wait_support(
> cond,
> mutex,
> + NULL,
> NULL
> );
> }
> diff --git a/cpukit/posix/src/condwaitsupp.c b/cpukit/posix/src/condwaitsupp.c
> index ee2f8a0787..93d6e1c9d1 100644
> --- a/cpukit/posix/src/condwaitsupp.c
> +++ b/cpukit/posix/src/condwaitsupp.c
> @@ -26,6 +26,7 @@
> #include <rtems/score/status.h>
> #include <rtems/score/threaddispatch.h>
>
> +
Added blank line. Remove.
> static void _POSIX_Condition_variables_Mutex_unlock(
> Thread_queue_Queue *queue,
> Thread_Control *the_thread,
> @@ -94,6 +95,7 @@ static void _POSIX_Condition_variables_Enqueue_with_timeout_realtime(
> int _POSIX_Condition_variables_Wait_support(
> pthread_cond_t *cond,
> pthread_mutex_t *mutex,
> + clockid_t clock_id,
> const struct timespec *abstime
Line it up.
> )
> {
> @@ -107,28 +109,58 @@ int _POSIX_Condition_variables_Wait_support(
> POSIX_CONDITION_VARIABLES_VALIDATE_OBJECT( the_cond, flags );
>
> _Thread_queue_Context_initialize( &queue_context );
> -
> - if ( abstime != NULL ) {
> - _Thread_queue_Context_set_timeout_argument( &queue_context, abstime, true );
> -
> - if ( _POSIX_Condition_variables_Get_clock( flags ) == CLOCK_MONOTONIC ) {
> +
> + /* If there is a clock_id parameter, this is a call to
> + * pthread_cond_clockwait. Set the clock according to
> + * this parameter. */
This can be pulled up to two lines I think and stay under 80 columns.
> + if ( clock_id ) {
> + _Thread_queue_Context_set_timeout_argument( &queue_context,
> + abstime, true );
Is this really too long? If it is, this isn't the pattern for breaking the line.
> +
> + /* We have already validated supported clocks in condclockwait.c.
> + * if clock_id is not clock_monotonic, then it is clock_realtime. */
Pull the closing */ down to its own line.
Capitalize CLOCK_MONOTONIC and CLOCK_REALTIME since you are
using their proper name.
> + if ( clock_id == CLOCK_MONOTONIC ) {
> _Thread_queue_Context_set_enqueue_callout(
> &queue_context,
> _POSIX_Condition_variables_Enqueue_with_timeout_monotonic
> );
> - } else {
> - _Thread_queue_Context_set_enqueue_callout(
> + }
> +
> + else {
> + _Thread_queue_Context_set_enqueue_callout(
> &queue_context,
> _POSIX_Condition_variables_Enqueue_with_timeout_realtime
> );
> }
> - } else {
> - _Thread_queue_Context_set_enqueue_callout(
> - &queue_context,
> - _POSIX_Condition_variables_Enqueue_no_timeout
> - );
> }
>
> + /* If there is no clock_id parameter, this is either a call to
> + * cond_timedwait or cond_wait. */
> + else {
Pull the */ down if this has to be on multiple lines.
Go ahead and put {} around the true condition that we only see the
else { for. This looks like a case where that would add clarity.
NOTE: It is likely we should be better about having {} more frequently.
> +
> + /* If there is an abstime parameter, this is a call to cond_timedwait. */
> + if ( abstime != NULL ) {
> + _Thread_queue_Context_set_timeout_argument( &queue_context, abstime, true );
> +
> + if ( _POSIX_Condition_variables_Get_clock( flags ) == CLOCK_MONOTONIC ) {
> + _Thread_queue_Context_set_enqueue_callout(
> + &queue_context,
> + _POSIX_Condition_variables_Enqueue_with_timeout_monotonic
> + );
> + } else {
> + _Thread_queue_Context_set_enqueue_callout(
> + &queue_context,
> + _POSIX_Condition_variables_Enqueue_with_timeout_realtime
> + );
> + }
> + /* If there is no abstime parameter, this is a call to cond_wait. */
> + } else {
> + _Thread_queue_Context_set_enqueue_callout(
> + &queue_context,
> + _POSIX_Condition_variables_Enqueue_no_timeout
> + );
> + }
> + }
Did this just get reformatted as part of adding the support? It seems
to be the same.
> executing = _POSIX_Condition_variables_Acquire( the_cond, &queue_context );
>
> if (
> @@ -138,9 +170,8 @@ int _POSIX_Condition_variables_Wait_support(
> _POSIX_Condition_variables_Release( the_cond, &queue_context );
> return EINVAL;
> }
> -
> +
What happened here? Was there a space before the end of line or did you
add a space?
> the_cond->mutex = mutex;
> -
> _Thread_queue_Context_set_thread_state(
> &queue_context,
> STATES_WAITING_FOR_CONDITION_VARIABLE
> @@ -151,8 +182,9 @@ int _POSIX_Condition_variables_Wait_support(
> executing,
> &queue_context
> );
> +
I suspect this blank line is OK since it probably makes the code
look better.
> error = _POSIX_Get_error_after_wait( executing );
> -
> +
Again with odd change.
> /*
> * If the thread is interrupted, while in the thread queue, by
> * a POSIX signal, then pthread_cond_wait returns spuriously,
> @@ -177,7 +209,7 @@ int _POSIX_Condition_variables_Wait_support(
> _Assert( mutex_error == EINVAL );
> error = EINVAL;
> }
> - }
>
> + }
What is this change?
> return error;
> }
> diff --git a/spec/build/testsuites/psxtests/grp.yml b/spec/build/testsuites/psxtests/grp.yml
> index fb7ce465ae..fe945fe7b8 100644
> --- a/spec/build/testsuites/psxtests/grp.yml
> +++ b/spec/build/testsuites/psxtests/grp.yml
> @@ -47,6 +47,8 @@ links:
> uid: psx15
> - role: build-dependency
> uid: psx16
> +- role: build-dependency
> + uid: psx17
> - role: build-dependency
> uid: psxaio01
> - role: build-dependency
> diff --git a/spec/build/testsuites/psxtests/psx17.yml b/spec/build/testsuites/psxtests/psx17.yml
> new file mode 100644
> index 0000000000..94f7567fc6
> --- /dev/null
> +++ b/spec/build/testsuites/psxtests/psx17.yml
> @@ -0,0 +1,20 @@
> +SPDX-License-Identifier: CC-BY-SA-4.0 OR BSD-2-Clause
> +build-type: test-program
> +cflags: []
> +copyrights:
> +- Copyright (C) 2020 embedded brains GmbH (http://www.embedded-brains.de)
> +cppflags: []
> +cxxflags: []
> +enabled-by: true
> +features: c cprogram
> +includes: []
> +ldflags: []
> +links: []
> +source:
> +- testsuites/psxtests/psx17/init.c
> +- cpukit/posix/src/condclockwait.c
there is already psxcond01 and 02. I would make this psxcond03 so it
has a name that more clearly shows its area.
> +stlib: []
> +target: testsuites/psxtests/psx17.exe
> +type: build
> +use-after: []
> +use-before: []
> diff --git a/testsuites/psxtests/Makefile.am b/testsuites/psxtests/Makefile.am
> index a35f00b665..a8d2351e49 100755
> --- a/testsuites/psxtests/Makefile.am
> +++ b/testsuites/psxtests/Makefile.am
> @@ -162,6 +162,14 @@ psx16_CPPFLAGS = $(AM_CPPFLAGS) $(TEST_FLAGS_psx16) \
> $(support_includes) -I$(top_srcdir)/include
> endif
>
> +if TEST_psx17
> +psx_tests += psx17
> +psx_screens += psx17/psx17.scn
> +psx17_SOURCES = psx17/init.c psx17/system.h include/pmacros.h
> +psx17_CPPFLAGS = $(AM_CPPFLAGS) $(TEST_FLAGS_psx17) \
> + $(support_includes) -I$(top_srcdir)/include
> +endif
> +
> if HAS_POSIX
> if TEST_psxaio01
> psx_tests += psxaio01
> diff --git a/testsuites/psxtests/configure.ac b/testsuites/psxtests/configure.ac
> index 3f95010cd3..cdfa349ae4 100644
> --- a/testsuites/psxtests/configure.ac
> +++ b/testsuites/psxtests/configure.ac
> @@ -59,6 +59,7 @@ RTEMS_TEST_CHECK([psx13])
> RTEMS_TEST_CHECK([psx14])
> RTEMS_TEST_CHECK([psx15])
> RTEMS_TEST_CHECK([psx16])
> +RTEMS_TEST_CHECK([psx17])
> RTEMS_TEST_CHECK([psxaio01])
> RTEMS_TEST_CHECK([psxaio02])
> RTEMS_TEST_CHECK([psxaio03])
> diff --git a/testsuites/psxtests/psx17/init.c b/testsuites/psxtests/psx17/init.c
> new file mode 100644
> index 0000000000..bc284af94d
> --- /dev/null
> +++ b/testsuites/psxtests/psx17/init.c
> @@ -0,0 +1,425 @@
> +/*
> +* Copyright (C) 2021 Matthew Joyce
> +*
> +* Redistribution and use in source and binary forms, with or without
> +* modification, are permitted provided that the following conditions
> +* are met:
> +* 1. Redistributions of source code must retain the above copyright
> +* notice, this list of conditions and the following disclaimer.
> +* 2. Redistributions in binary form must reproduce the above copyright
> +* notice, this list of conditions and the following disclaimer in the
> +* documentation and/or other materials provided with the distribution.
> +*
> +* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
> +* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> +* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> +* ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
> +* LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> +* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> +* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> +* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> +* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> +* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> +* POSSIBILITY OF SUCH DAMAGE.
> +*/
> +
> +/* Defining in order to access pthread_cond_clockwait in pthread.h */
> +#define _GNU_SOURCE
> +
> +#ifdef HAVE_CONFIG_H
> +#include "config.h"
> +#endif
> +
> +#define CONFIGURE_INIT
> +#include <pthread.h>
> +#include <sched.h>
> +#include <errno.h>
> +#include <rtems.h>
> +#include <tmacros.h>
> +#include "system.h"
> +
> +pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
> +pthread_cond_t bad_cond;
> +pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
> +pthread_mutex_t bad_mutex;
> +pthread_mutex_t new_mutex = PTHREAD_MUTEX_INITIALIZER;
> +clockid_t clock_id1 = CLOCK_MONOTONIC;
> +clockid_t clock_id2 = CLOCK_REALTIME;
> +clockid_t clock_id3 = CLOCK_MONOTONIC_RAW;
> +int count;
> +int eno;
> +int status;
> +const char rtems_test_name[] = "PSX 17";
> +
> +void *POSIX_Init(
> + void *argument
> +)
> +{
> + TEST_BEGIN();
> + empty_line();
> + pthread_t t1;
> + struct timespec abstime;
> +
> + /* Expected pass: Clock Monotonic with sufficient abstime */
> + printf("1st iteration Begin (Clock Monotonic)\n");
> + abstime.tv_sec = 2;
> + abstime.tv_nsec = 1;
> + status = pthread_create( &t1, NULL, thread_func, NULL );
> + if ( status != 0 ) {
> + perror( "pthread_create" );
> + }
> + pthread_mutex_lock(&mutex);
> + while ( count == 0 ){
> + eno = pthread_cond_clockwait( &cond, &mutex, clock_id1, &abstime );
> + if ( eno != 0 ) {
> + break;
> + }
> + }
> +
> + count -= 1;
> + printf( "Minus called. Count is %d\n", count );
> + pthread_mutex_unlock( &mutex );
> + rtems_test_assert( eno == 0 );
> + printf( "Return value from cond_clockwait is %d\n", eno );
> + pthread_join( t1, NULL );
> + printf( "1st iteration END\n" );
> + empty_line();
> +
> + /* Expected fail: Clock Monotonic with insufficient abstime */
> + printf( "2nd iteration Begin (Clock Monotonic)\n" );
> + abstime.tv_sec = 0;
> + abstime.tv_nsec = 1;
> + status = pthread_create( &t1, NULL, thread_func, NULL );
> + if ( status != 0 ) {
> + perror( "pthread_create" );
> + }
> + pthread_mutex_lock( &mutex );
> + while ( count == 0 ){
> + eno = pthread_cond_clockwait( &cond, &mutex, clock_id1, &abstime );
> + if ( eno != 0 ) {
> + break;
> + }
> + }
> +
> + count -= 1;
> + printf( "Minus called. Count is %d\n", count );
> + pthread_mutex_unlock( &mutex );
> + rtems_test_assert( eno == ETIMEDOUT );
> + printf( "Return value from cond_clockwait is %d\n", eno );
> + pthread_join( t1, NULL );
> + printf( "2nd iteration END\n" );
> + empty_line();
> +
> + /* Expected pass: Clock Realtime with sufficient abstime */
> + printf( "3rd iteration Begin (Clock Realtime)\n" );
> + status = clock_gettime( clock_id2, &abstime );
> + rtems_test_assert( !status );
> + abstime.tv_sec += 4;
> + abstime.tv_nsec = 1;
> +
> + status = pthread_create( &t1, NULL, thread_func, NULL );
> + if ( status != 0 ) {
> + perror( "pthread_create" );
> + }
> + pthread_mutex_lock( &mutex );
> + while ( count == 0 ){
> + eno = pthread_cond_clockwait( &cond, &mutex, clock_id2, &abstime );
> + if ( eno != 0 ) {
> + break;
> + }
> + }
> +
> + count -= 1;
> + printf( "Minus called. Count is %d\n", count );
> + pthread_mutex_unlock( &mutex );
> + rtems_test_assert( eno == 0 );
> + printf( "Return value from cond_clockwait is %d\n", eno );
> + pthread_join( t1, NULL );
> + printf( "3rd iteration END\n" );
> + empty_line();
> +
> + /* Expected fail: Clock Realtime with insufficient abstime */
> + printf( "4th iteration Begin (Clock Realtime)\n" );
> + status = clock_gettime( clock_id2, &abstime );
> + rtems_test_assert( !status );
> + abstime.tv_sec += 0;
> + abstime.tv_nsec = 1;
> +
> + status = pthread_create( &t1, NULL, thread_func, NULL );
> + if ( status != 0 ) {
> + perror( "pthread_create" );
> + }
> + pthread_mutex_lock( &mutex );
> + while ( count == 0 ){
> + eno = pthread_cond_clockwait( &cond, &mutex, clock_id2, &abstime );
> + if ( eno != 0 ) {
> + break;
> + }
> + }
> +
> + count -= 1;
> + printf( "Minus called. Count is %d\n", count );
> + pthread_mutex_unlock( &mutex );
> + rtems_test_assert( eno == ETIMEDOUT );
> + printf( "Return value from cond_clockwait is %d\n", eno );
> + pthread_join( t1, NULL );
> + printf( "4th iteration END\n" );
> + empty_line();
> +
> + /* Expected fail: Unsupported Clock */
> + printf( "5th iteration Begin (Unsupported Clock)\n" );
> + abstime.tv_sec = 2;
> + abstime.tv_nsec = 1;
> + status = pthread_create( &t1, NULL, thread_func, NULL );
> + if ( status != 0 ) {
> + perror( "pthread_create" );
> + }
> + pthread_mutex_lock( &mutex );
> + while ( count == 0 ){
> + eno = pthread_cond_clockwait( &cond, &mutex, clock_id3, &abstime );
> + if ( eno != 0 ) {
> + break;
> + }
> + }
> +
> + count -= 1;
> + printf( "Minus called. Count is %d\n", count );
> + pthread_mutex_unlock( &mutex );
> + rtems_test_assert( eno == EINVAL );
> + printf( "Return value from cond_clockwait is %d\n", eno );
> + pthread_join( t1, NULL );
> + printf( "5th iteration END\n" );
> + empty_line();
> +
> + /* Expected fail: Invalid Clock */
> + printf( "6th iteration Begin (Invalid Clock)\n" );
> + abstime.tv_sec = 2;
> + abstime.tv_nsec = 1;
> + status = pthread_create( &t1, NULL, thread_func, NULL );
> + if ( status != 0 ) {
> + perror( "pthread_create" );
> + }
> + pthread_mutex_lock( &mutex );
> + while ( count == 0 ){
> + eno = pthread_cond_clockwait( &cond, &mutex, (int)NULL, &abstime );
> + if ( eno != 0 ) {
> + break;
> + }
> + }
> +
> + count -= 1;
> + printf( "Minus called. Count is %d\n", count );
> + pthread_mutex_unlock( &mutex );
> + rtems_test_assert( eno == EINVAL );
> + printf( "Return value from cond_clockwait is %d\n", eno );
> + pthread_join( t1, NULL );
> + printf( "6th iteration END\n" );
> + empty_line();
> +
> + /* Expected fail: Invalid Clock */
> + printf( "7th iteration Begin (Invalid Abstime)\n" );
> + abstime.tv_sec = 2;
> + abstime.tv_nsec = 1;
> + status = pthread_create( &t1, NULL, thread_func, NULL );
> + if ( status != 0 ) {
> + perror( "pthread_create" );
> + }
> + pthread_mutex_lock( &mutex );
> + while ( count == 0 ){
> + eno = pthread_cond_clockwait( &cond, &mutex, clock_id1, NULL );
> + if ( eno != 0 ) {
> + break;
> + }
> + }
> +
> + count -= 1;
> + printf( "Minus called. Count is %d\n", count );
> + pthread_mutex_unlock( &mutex );
> + rtems_test_assert( eno == EINVAL );
> + printf( "Return value from cond_clockwait is %d\n", eno );
> + pthread_join( t1, NULL );
> + printf( "7th iteration END\n" );
> + empty_line();
> +
> + /* Expected fail: Invalid Clock */
> + printf( "8th iteration Begin (Invalid Cond)\n" );
> + abstime.tv_sec = 2;
> + abstime.tv_nsec = 1;
> + status = pthread_create( &t1, NULL, thread_func, NULL );
> + if ( status != 0 ) {
> + perror( "pthread_create" );
> + }
> + pthread_mutex_lock( &mutex );
> + while ( count == 0 ){
> + eno = pthread_cond_clockwait( NULL, &mutex, clock_id1, &abstime );
> + if ( eno != 0 ) {
> + break;
> + }
> + }
> +
> + count -= 1;
> + printf( "Minus called. Count is %d\n", count );
> + pthread_mutex_unlock( &mutex );
> + rtems_test_assert( eno == EINVAL );
> + printf( "Return value from cond_clockwait is %d\n", eno );
> + pthread_join( t1, NULL );
> + printf( "8th iteration END\n" );
> + empty_line();
> +
> + /* Expected fail: Invalid Mutex */
> + printf( "9th iteration Begin (Invalid Mutex)\n" );
> + abstime.tv_sec = 2;
> + abstime.tv_nsec = 1;
> + status = pthread_create( &t1, NULL, thread_func, NULL );
> + if ( status != 0 ) {
> + perror( "pthread_create" );
> + }
> + pthread_mutex_lock( &mutex );
> + while ( count == 0 ){
> + eno = pthread_cond_clockwait( &cond, NULL, clock_id1, &abstime );
> + if ( eno != 0 ) {
> + break;
> + }
> + }
> +
> + count -= 1;
> + printf( "Minus called. Count is %d\n", count );
> + pthread_mutex_unlock( &mutex );
> + rtems_test_assert( eno != 0 );
> + printf( "Return value from cond_clockwait is %d\n", eno );
> + pthread_join( t1, NULL );
> + printf( "9th iteration END\n" );
> + empty_line();
> +
> + /* Expected fail: Uninitialized condition variable */
> + printf( "10th iteration Begin (Uninitialized Condition Variable)\n" );
> + abstime.tv_sec = 2;
> + abstime.tv_nsec = 1;
> + status = pthread_create( &t1, NULL, thread_func, NULL );
> + if ( status != 0 ) {
> + perror( "pthread_create" );
> + }
> + pthread_mutex_lock( &mutex );
> + while ( count == 0 ){
> + eno = pthread_cond_clockwait( &bad_cond, &mutex, clock_id1, &abstime );
> + if ( eno != 0 ) {
> + break;
> + }
> + }
> +
> + count -= 1;
> + printf( "Minus called. Count is %d\n", count );
> + pthread_mutex_unlock( &mutex );
> + rtems_test_assert( eno != 0 );
> + printf( "Return value from cond_clockwait is %d\n", eno );
> + pthread_join( t1, NULL );
> + printf( "10th iteration END\n" );
> + empty_line();
> +
> + /* Expected fail: Uninitialized condition variable */
> + printf( "11th iteration Begin (Uninitialized Mutex)\n" );
> + abstime.tv_sec = 2;
> + abstime.tv_nsec = 1;
> + status = pthread_create( &t1, NULL, thread_func, NULL );
> + if ( status != 0 ) {
> + perror( "pthread_create" );
> + }
> + pthread_mutex_lock( &mutex );
> + while ( count == 0 ){
> + eno = pthread_cond_clockwait( &cond, &bad_mutex, clock_id1, &abstime );
> + if ( eno != 0 ) {
> + break;
> + }
> + }
> +
> + count -= 1;
> + printf( "Minus called. Count is %d\n", count );
> + pthread_mutex_unlock( &mutex );
> + rtems_test_assert( eno != 0 );
> + printf( "Return value from cond_clockwait is %d\n", eno );
> + pthread_join( t1, NULL );
> + printf( "11th iteration END\n" );
> + empty_line();
> +
> + /* Expected fail: Uninitialized condition variable */
> + printf( "10th iteration Begin (Uninitialized Condition Variable)\n" );
> + abstime.tv_sec = 2;
> + abstime.tv_nsec = 1;
> + status = pthread_create( &t1, NULL, thread_func, NULL );
> + if ( status != 0 ) {
> + perror( "pthread_create" );
> + }
> + pthread_mutex_lock( &mutex );
> + while ( count == 0 ){
> + eno = pthread_cond_clockwait( &bad_cond, &mutex, clock_id1, &abstime );
> + if ( eno != 0 ) {
> + break;
> + }
> + }
> +
> + count -= 1;
> + printf( "Minus called. Count is %d\n", count );
> + pthread_mutex_unlock( &mutex );
> + rtems_test_assert( eno != 0 );
> + printf( "Return value from cond_clockwait is %d\n", eno );
> + pthread_join( t1, NULL );
> + printf( "10th iteration END\n" );
> + empty_line();
> +
> + /* Expected pass: Binding new mutex to condition variable */
> + printf( "12th iteration Begin (New Mutex)\n" );
> + status = clock_gettime( clock_id2, &abstime );
> + rtems_test_assert( !status );
> + abstime.tv_sec += 4;
> + abstime.tv_nsec = 1;
> + status = pthread_create( &t1, NULL, thread_func2, NULL );
> + if ( status != 0 ) {
> + perror( "pthread_create" );
> + }
> + pthread_mutex_lock( &new_mutex );
> + while ( count == 0 ){
> + eno = pthread_cond_clockwait( &cond, &new_mutex, clock_id2, &abstime );
> + if ( eno != 0 ) {
> + break;
> + }
> + }
> +
> + count -= 1;
> + printf( "Minus called. Count is %d\n", count );
> + pthread_mutex_unlock( &new_mutex );
> + rtems_test_assert( eno == 0 );
> + printf( "Return value from cond_clockwait is %d\n", eno );
> + pthread_join( t1, NULL );
> + printf( "12th iteration END\n" );
> + empty_line();
> +
> + TEST_END();
> + rtems_test_exit( 0 );
> + return NULL;
> +}
> +
> +void *thread_func( void *arg )
> +{
> + pthread_mutex_lock( &mutex );
> + if (count == 0) {
> + pthread_cond_signal( &cond );
> + }
> +
> + count +=1;
> + printf( "Add called. Count is %d\n", count );
> + pthread_mutex_unlock( &mutex );
> + return NULL;
> +}
> +
> +void *thread_func2( void *arg )
> +{
> + pthread_mutex_lock( &new_mutex );
> + if ( count == 0 ) {
> + pthread_cond_signal( &cond );
> + }
> +
> + count +=1;
> + printf( "Add called. Count is %d\n", count );
> + pthread_mutex_unlock( &new_mutex );
> + return NULL;
> +}
Is this a copy of an existing test with changes to the cond wait call? If so,
it would be better to conditionally compile the differences. There are other
tests which use the same code with a minor change to test two similar
APIs.
If this is a new test, it needs to self check and print less. I see
places where it
does not check return codes. It should use test assertion macros on all
return codes. The test automation does not compare the screen output
with the actual run. The scn is a guide for developers on what to expect.
But all checks for behavior as expected re internal to the test.
> diff --git a/testsuites/psxtests/psx17/psx17.doc b/testsuites/psxtests/psx17/psx17.doc
> new file mode 100644
> index 0000000000..d1206982c9
> --- /dev/null
> +++ b/testsuites/psxtests/psx17/psx17.doc
> @@ -0,0 +1,45 @@
> +/*
> +* Copyright (C) 2021 Matthew Joyce
> +*
> +* Redistribution and use in source and binary forms, with or without
> +* modification, are permitted provided that the following conditions
> +* are met:
> +* 1. Redistributions of source code must retain the above copyright
> +* notice, this list of conditions and the following disclaimer.
> +* 2. Redistributions in binary form must reproduce the above copyright
> +* notice, this list of conditions and the following disclaimer in the
> +* documentation and/or other materials provided with the distribution.
> +*
> +* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
> +* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> +* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> +* ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
> +* LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> +* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> +* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> +* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> +* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> +* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> +* POSSIBILITY OF SUCH DAMAGE.
> +*/
> +
> +This file describes the directives and concepts tested by this test set.
> +
> +test set name: psx17
> +
> +directives:
> +
> + pthread_cond_clockwait
> + pthread_create
> + pthread_mutex_lock
> + pthread_mutex_unlock
> + pthread_cond_signal
> + pthread_join
> +
> +concepts:
> +
> ++ Tests the newly-added, Issue 8 POSIX Standard pthread_cond_clockwait method.
> + Tests include valid/supported and invalid/unsupported clocks, sufficient and
> + insufficient timeouts, and invalid or uninitialized condition/mutex
> + parameters.
> +
> \ No newline at end of file
> diff --git a/testsuites/psxtests/psx17/psx17.scn b/testsuites/psxtests/psx17/psx17.scn
> new file mode 100644
> index 0000000000..d3e7fd024f
> --- /dev/null
> +++ b/testsuites/psxtests/psx17/psx17.scn
> @@ -0,0 +1,82 @@
> +*** BEGIN OF TEST PSX 17 ***
> +
> +1st iteration Begin (Clock Monotonic)
> +Add called. Count is 1
> +Minus called. Count is 0
> +Return value from cond_clockwait is 0
> +1st iteration END
> +
> +2nd iteration Begin (Clock Monotonic)
> +Add called. Count is 1
> +Minus called. Count is 0
> +Return value from cond_clockwait is 116
> +2nd iteration END
> +
> +3rd iteration Begin (Clock Realtime)
> +Add called. Count is 1
> +Minus called. Count is 0
> +Return value from cond_clockwait is 0
> +3rd iteration END
> +
> +4th iteration Begin (Clock Realtime)
> +Add called. Count is 1
> +Minus called. Count is 0
> +Return value from cond_clockwait is 116
> +4th iteration END
> +
> +5th iteration Begin (Unsupported Clock)
> +Minus called. Count is -1
> +Return value from cond_clockwait is 22
> +Add called. Count is 0
> +5th iteration END
> +
> +6th iteration Begin (Invalid Clock)
> +Minus called. Count is -1
> +Return value from cond_clockwait is 22
> +Add called. Count is 0
> +6th iteration END
> +
> +7th iteration Begin (Invalid Abstime)
> +Minus called. Count is -1
> +Return value from cond_clockwait is 22
> +Add called. Count is 0
> +7th iteration END
> +
> +8th iteration Begin (Invalid Cond)
> +Minus called. Count is -1
> +Return value from cond_clockwait is 22
> +Add called. Count is 0
> +8th iteration END
> +
> +9th iteration Begin (Invalid Mutex)
> +Minus called. Count is -1
> +Return value from cond_clockwait is 1
> +Add called. Count is 0
> +9th iteration END
> +
> +10th iteration Begin (Uninitialized Condition Variable)
> +Add called. Count is 1
> +Minus called. Count is 0
> +Return value from cond_clockwait is 116
> +10th iteration END
> +
> +11th iteration Begin (Uninitialized Mutex)
> +Minus called. Count is -1
> +Return value from cond_clockwait is 116
> +Add called. Count is 0
> +11th iteration END
> +
> +10th iteration Begin (Uninitialized Condition Variable)
> +Add called. Count is 1
> +Minus called. Count is 0
> +Return value from cond_clockwait is 116
> +10th iteration END
> +
> +12th iteration Begin (New Mutex)
> +Add called. Count is 1
> +Minus called. Count is 0
> +Return value from cond_clockwait is 0
> +12th iteration END
> +
> +
> +*** END OF TEST PSX 17 ***
> diff --git a/testsuites/psxtests/psx17/system.h b/testsuites/psxtests/psx17/system.h
> new file mode 100644
> index 0000000000..7da50cca7f
> --- /dev/null
> +++ b/testsuites/psxtests/psx17/system.h
> @@ -0,0 +1,57 @@
> +/*
> +* Copyright (C) 2021 Matthew Joyce
> +*
> +* Redistribution and use in source and binary forms, with or without
> +* modification, are permitted provided that the following conditions
> +* are met:
> +* 1. Redistributions of source code must retain the above copyright
> +* notice, this list of conditions and the following disclaimer.
> +* 2. Redistributions in binary form must reproduce the above copyright
> +* notice, this list of conditions and the following disclaimer in the
> +* documentation and/or other materials provided with the distribution.
> +*
> +* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
> +* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> +* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> +* ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
> +* LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> +* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> +* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> +* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> +* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> +* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> +* POSSIBILITY OF SUCH DAMAGE.
> +*/
> +
> +/* functions */
> +
> +#include <pmacros.h>
> +#include <unistd.h>
> +#include <errno.h>
> +
> +void *POSIX_Init(
> + void *argument
> +);
> +
> +void *thread_func(
> + void *argument
> +);
> +
> +void *thread_func2(
> + void *argument
> +);
> +
> +/* configuration information */
> +
> +#define CONFIGURE_APPLICATION_NEEDS_SIMPLE_CONSOLE_DRIVER
> +#define CONFIGURE_APPLICATION_NEEDS_CLOCK_DRIVER
> +
> +#define CONFIGURE_INITIAL_EXTENSIONS RTEMS_TEST_INITIAL_EXTENSION
> +
> +#define CONFIGURE_MAXIMUM_POSIX_THREADS 4
> +
> +#define CONFIGURE_POSIX_INIT_THREAD_TABLE
> +
> +#include <rtems/confdefs.h>
> +
> +/* end of include file */
> diff --git a/testsuites/psxtests/psxhdrs/pthread/pthread_cond_clockwait.c b/testsuites/psxtests/psxhdrs/pthread/pthread_cond_clockwait.c
> index 15485eb587..fe91b7954f 100644
> --- a/testsuites/psxtests/psxhdrs/pthread/pthread_cond_clockwait.c
> +++ b/testsuites/psxtests/psxhdrs/pthread/pthread_cond_clockwait.c
> @@ -49,7 +49,7 @@ int test( void )
> pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
> clockid_t clock_id = CLOCK_REALTIME;
> struct timespec abstime;
> - abstime.tv_sec = 2;
> + abstime.tv_sec = 1;
Why this change?
> int result;
>
> /* This method appeared in the Issue 8 POSIX Standard */
> --
> 2.31.1
>
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
More information about the devel
mailing list