<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Apr 15, 2020 at 10:26 AM Sebastian Huber <<a href="mailto:sebastian.huber@embedded-brains.de" target="_blank">sebastian.huber@embedded-brains.de</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hello Utkarsh Rai,<br>
<br>
do we really need a new test program for this test case? I would prefer <br>
add it to an existing test program or add a generic POSIX test program <br>
using the RTEMS Test Framework.<br></blockquote><div>Would it be alright to add the test cases for clock nanosleep in psxtests/psxclock? I had two test cases in mind-</div><div>To check for a simple desired delay and to check that clock_nanosleep produces the required delay with CLOCK_MONOTONIC even when CLOCK_REALTIME is modified.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
On 14/04/2020 19:17, Utkarsh Rai wrote:<br>
> This test checks for a simple 1 ns delay with clock_nanosleep with<br>
> CLOCK_MONOTONIC.<br>
> ---<br>
>   testsuites/psxtests/Makefile.am               |  9 +++<br>
>   testsuites/psxtests/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a>              |  1 +<br>
>   .../psxtests/psxclocknanosleep01/init.c       | 81 +++++++++++++++++++<br>
>   .../psxclocknanosleep01.doc                   | 10 +++<br>
>   .../psxclocknanosleep01.scn                   |  3 +<br>
>   5 files changed, 104 insertions(+)<br>
>   create mode 100644 testsuites/psxtests/psxclocknanosleep01/init.c<br>
>   create mode 100644 testsuites/psxtests/psxclocknanosleep01/psxclocknanosleep01.doc<br>
>   create mode 100644 testsuites/psxtests/psxclocknanosleep01/psxclocknanosleep01.scn<br>
><br>
> diff --git a/testsuites/psxtests/Makefile.am b/testsuites/psxtests/Makefile.am<br>
> index 1f9e4233ec..e3918ae7a5 100755<br>
> --- a/testsuites/psxtests/Makefile.am<br>
> +++ b/testsuites/psxtests/Makefile.am<br>
> @@ -321,6 +321,15 @@ psxclockrealtime01_CPPFLAGS = $(AM_CPPFLAGS) \<br>
>       $(TEST_FLAGS_psxclockrealtime01) $(support_includes)<br>
>   endif<br>
>   <br>
> +if TEST_psxclocknanosleep01<br>
> +psx_tests += psxclocknanosleep01<br>
> +psx_screens += psxclocknanosleep01/psxclocknanosleep01.scn<br>
> +psx_docs += psxclocknanosleep01/psxclocknanosleep01.doc<br>
> +psxclocknanosleep01_SOURCES = psxclocknanosleep01/init.c<br>
> +psxclocknanosleep01_CPPFLAGS = $(AM_CPPFLAGS) \<br>
> +     $(TEST_FLAGS_psxclocknanosleep01) $(support_includes)<br>
> +endif<br>
> +<br>
>   if TEST_psxconcurrency01<br>
>   psx_tests += psxconcurrency01<br>
>   psx_screens += psxconcurrency01/psxconcurrency01.scn<br>
> diff --git a/testsuites/psxtests/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a> b/testsuites/psxtests/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a><br>
> index 139787cccb..9bfe8e2c0b 100644<br>
> --- a/testsuites/psxtests/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a><br>
> +++ b/testsuites/psxtests/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a><br>
> @@ -75,6 +75,7 @@ RTEMS_TEST_CHECK([psxcleanup01])<br>
>   RTEMS_TEST_CHECK([psxcleanup02])<br>
>   RTEMS_TEST_CHECK([psxclock])<br>
>   RTEMS_TEST_CHECK([psxclock01])<br>
> +RTEMS_TEST_CHECK([psxclocknanosleep01])<br>
>   RTEMS_TEST_CHECK([psxclockrealtime01])<br>
>   RTEMS_TEST_CHECK([psxconcurrency01])<br>
>   RTEMS_TEST_CHECK([psxcond01])<br>
> diff --git a/testsuites/psxtests/psxclocknanosleep01/init.c b/testsuites/psxtests/psxclocknanosleep01/init.c<br>
> new file mode 100644<br>
> index 0000000000..21b738627d<br>
> --- /dev/null<br>
> +++ b/testsuites/psxtests/psxclocknanosleep01/init.c<br>
> @@ -0,0 +1,81 @@<br>
> +/*<br>
> + * SPDX-License-Identifier: BSD-2-Clause<br>
> + *<br>
> + * Copyright (C) 2020 Utkarsh Rai<br>
> + *<br>
> + * Redistribution and use in source and binary forms, with or without<br>
> + * modification, are permitted provided that the following conditions<br>
> + * are met:<br>
> + * 1. Redistributions of source code must retain the above copyright<br>
> + *    notice, this list of conditions and the following disclaimer.<br>
> + * 2. Redistributions in binary form must reproduce the above copyright<br>
> + *    notice, this list of conditions and the following disclaimer in the<br>
> + *    documentation and/or other materials provided with the distribution.<br>
> + *<br>
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"<br>
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE<br>
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE<br>
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE<br>
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR<br>
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF<br>
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS<br>
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN<br>
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)<br>
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE<br>
> + * POSSIBILITY OF SUCH DAMAGE.<br>
> + */<br>
> +<br>
> +#include <rtems.h><br>
> +#include <rtems/test.h><br>
> +#include <time.h><br>
> +#include <tmacros.h><br>
> +<br>
> +/*Forward declaration to avoid compiler warning*/<br>
> +void clock_sleep(void);<br>
Please use a static function instead. Most of these forward declarations <br>
are just lazy warning fixes and bad examples.<br>
> +<br>
> +rtems_task Init(rtems_task_argument ignored);<br>
> +<br>
> +const char rtems_test_name[] = "PSXCLOCKNANOSLEEP 01";<br>
> +<br>
> +void clock_sleep(void)<br>
> +{<br>
> +  struct timespec delay_time;<br>
> +  int    status;<br>
> +<br>
> +  delay_time.tv_sec = 0;<br>
> +  delay_time.tv_nsec = 1;<br>
> +<br>
> +  status=clock_nanosleep( CLOCK_MONOTONIC, 0, &delay_time, (struct timespec* )NULL );<br>
In C code this cast is superfluous, in C++ code using NULL is out dated.<br>
> +  rtems_test_assert( status == 0 );<br>
> +}<br>
> +<br>
> +rtems_task Init(rtems_task_argument ignored)<br>
> +{<br>
> +<br>
> +  struct timespec init_time;<br>
> +  struct timespec end_time;<br>
> +  int    status;<br>
> +<br>
> +  TEST_BEGIN();<br>
> +<br>
> +  status = clock_gettime( CLOCK_MONOTONIC, &init_time );<br>
> +  rtems_test_assert( status == 0 );<br>
> +<br>
> +  clock_sleep();<br>
> +<br>
> +  status = clock_gettime( CLOCK_MONOTONIC, &end_time );<br>
> +  rtems_test_assert( status == 0 );<br>
> +<br>
> +  rtems_test_assert( (end_time.tv_sec-init_time.tv_sec) == 0 );<br>
<br>
Is end_time.tv_sec - init_time.tv_sec == 0 under all circumstances?<br></blockquote><div><br></div><div>My idea was to check for a 1ns delay with a reasonable amount of overhead, hence I checked for  end_time.tv_sec - init_time.tv_sec == 0.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
The test case code should be separated from the test suite boilerplate code.<br>
<br>
> +  rtems_test_assert( (end_time.tv_nsec-init_time.tv_nsec) >=1 );<br>
We really should think about using a C code formatter. I think that I <br>
waste my time telling someone to put spaces between operators.<br></blockquote><div>Sorry, I missed this one, will not happen again. </div></div></div>