Re: [PATCH 1/1] Implementation / tests for pthread_cond_clockwait()
On Wed, Aug 18, 2021 at 8:44 PM Joel Sherrill wrote: > > On Wed, Aug 18, 2021 at 6:17 AM Matt Joyce 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. I'm having a little trouble with this and the one below: It looks aligned in both vscode and in vim...even when I cat the patch. But here it's clearly misaligned. I went back in and re-aligned all the variables, but I'm still getting this misaligned output. Could you please advise? > > > ); > > > > diff --git a/cpukit/posix/src/condclockwait.c > > b/cpukit/posix/src/condclockwait.c > > new file mode 100644 > > index 00..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 > > +#include > > + > > +/* > > + * 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
Re: [PATCH 1/1] Implementation / tests for pthread_cond_clockwait()
On Wed, Aug 18, 2021 at 6:17 AM Matt Joyce 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 00..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
> +#include
> +
> +/*
> + * 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
[PATCH 1/1] Implementation / tests for pthread_cond_clockwait()
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
);
diff --git a/cpukit/posix/src/condclockwait.c b/cpukit/posix/src/condclockwait.c
new file mode 100644
index 00..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
+*
+* 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
+#include
+
+/*
+ * 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,
+ const struct timespec *restrict abstime
+)
+{
+ if ( abstime == NULL ) {
+return EINVAL; /* not specified */
+ }
+
+ if ( clock_id != CLOCK_REALTIME && clock_id != CLOCK_MONOTONIC ) {
+return EINVAL; /* not specified */
+ }
+
+ 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(
