On Mon, Apr 13, 2020 at 10:26 PM Joel Sherrill <j...@rtems.org> wrote:
> I am pretty sure I can report that it won't execute on just visually > reviewing it. > > First, it assumes at least a semi-functional pthread_getcpuclockid() and > there is not one in the tree. > I have tested it against this patch https://lists.rtems.org/pipermail/devel/2020-March/058176.html and It works fine > > You should be testing things yourself. There are multiple simulators which > have BSPs and are easy to run. We can discuss which one in another thread. > Tested it on sparc/erc32 > Other comments interspersed. > > On Thu, Apr 9, 2020 at 3:30 PM Eshan dhawan <eshandhawa...@gmail.com> > wrote: > >> --- >> testsuites/psxtests/Makefile.am | 10 ++ >> testsuites/psxtests/configure.ac | 1 + >> testsuites/psxtests/psxgetcpuclockid01/init.c | 120 ++++++++++++++++++ >> .../psxgetcpuclockid01/psxgetcpuclockid01.doc | 19 +++ >> .../psxgetcpuclockid01/psxgetcpuclockid01.scn | 4 + >> 5 files changed, 154 insertions(+) >> create mode 100644 testsuites/psxtests/psxgetcpuclockid01/init.c >> create mode 100644 >> testsuites/psxtests/psxgetcpuclockid01/psxgetcpuclockid01.doc >> create mode 100644 >> testsuites/psxtests/psxgetcpuclockid01/psxgetcpuclockid01.scn >> >> diff --git a/testsuites/psxtests/Makefile.am >> b/testsuites/psxtests/Makefile.am >> index 1f9e4233ec..ffadff16e7 100755 >> --- a/testsuites/psxtests/Makefile.am >> +++ b/testsuites/psxtests/Makefile.am >> @@ -462,6 +462,16 @@ psxgetattrnp01_CPPFLAGS = $(AM_CPPFLAGS) >> $(TEST_FLAGS_psxgetattrnp01) \ >> $(support_includes) -I$(top_srcdir)/include >> endif >> >> +if TEST_psxgetcpuclockid01 >> +psx_tests += psxgetcpuclockid01 >> +psx_screens += psxgetcpuclockid01/psxgetcpuclockid01.scn >> +psx_docs += psxgetcpuclockid01/psxgetcpuclockid01.doc >> +psxgetcpuclockid01_SOURCES = psxgetcpuclockid01/init.c >> +psxgetcpuclockid01_CPPFLAGS = $(AM_CPPFLAGS) >> $(TEST_FLAGS_psxgetcpuclockid01) \ >> + $(support_includes) >> +endif >> + >> + >> if TEST_psxgetrusage01 >> psx_tests += psxgetrusage01 >> psx_screens += psxgetrusage01/psxgetrusage01.scn >> diff --git a/testsuites/psxtests/configure.ac b/testsuites/psxtests/ >> configure.ac >> index 139787cccb..c509086abc 100644 >> --- a/testsuites/psxtests/configure.ac >> +++ b/testsuites/psxtests/configure.ac >> @@ -91,6 +91,7 @@ RTEMS_TEST_CHECK([psxfile01]) >> RTEMS_TEST_CHECK([psxfile02]) >> RTEMS_TEST_CHECK([psxfilelock01]) >> RTEMS_TEST_CHECK([psxgetattrnp01]) >> +RTEMS_TEST_CHECK([psxgetcpuclockid01]) >> RTEMS_TEST_CHECK([psxgetrusage01]) >> RTEMS_TEST_CHECK([psxglobalcon01]) >> RTEMS_TEST_CHECK([psxglobalcon02]) >> diff --git a/testsuites/psxtests/psxgetcpuclockid01/init.c >> b/testsuites/psxtests/psxgetcpuclockid01/init.c >> new file mode 100644 >> index 0000000000..8e1093e44b >> --- /dev/null >> +++ b/testsuites/psxtests/psxgetcpuclockid01/init.c >> @@ -0,0 +1,120 @@ >> +/* >> + * @file >> + * @brief Test suite for getcpuclockid methods >> + */ >> + >> +/* >> + * SPDX-License-Identifier: BSD-2-Clause >> + * >> + * Copyright (C) 2020 Eshan Dhawan >> + * >> + * 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. >> + */ >> + >> + >> +#ifdef HAVE_CONFIG_H >> +#include "config.h" >> +#endif >> + >> +#include <errno.h> >> +#include <pthread.h> >> +#include <stdint.h> >> +#include <rtems/test.h> >> +#include <sys/utsname.h> >> +#include <unistd.h> >> +#include <errno.h> >> +#include <sched.h> >> >> I am sure at least some of these are not needed. I suspect almost > everything below rtems/test.h is not needed. > > errno.h is twice. > I will correct that > > >> +#include <rtems/score/todimpl.h> >> +#include <rtems/score/threadimpl.h> >> > > Why are these needed? > > >> +#include "tmacros.h" >> + >> +const char rtems_test_name[] = "PSXGETCPUCLOCKID 1"; >> + >> +/* Forward declaration to avoid warnings */ >> +rtems_task Init(rtems_task_argument ignored); >> +void *test_pthread(void *); >> +/* test_task function begins */ >> +void *test_pthread(void *arg) >> +{ >> + int r; >> + clockid_t cid; >> + >> + r = pthread_getcpuclockid( pthread_self(), &cid ); >> + if(r) >> + { >> + printf( "pthread_getcpuclockid : 0x%d", r ); >> + } >> + rtems_test_assert( r == 0 ); >> + >> + pthread_exit(0); >> + return NULL; >> +} >> + >> +/* init test function begins */ >> +rtems_task Init(rtems_task_argument ignored) >> +{ >> > > For POSIX tests, use a POSIX Initialization thread unless specifically > verifying that a Classic API task has particular behavior when using POSIX > methods. > Ok > > >> + TEST_BEGIN(); >> + >> + int r; >> + clockid_t cid; >> + pthread_t thread_id; >> + >> + r= pthread_create( &thread_id, NULL, test_pthread, NULL ); >> + if( r ) >> + { >> + printf( "pthread_create : 0x%d", r ); >> + } >> + rtems_test_assert( r == 0 ); >> > > > First, "0x%d" will print a 0x saying it is hex and then print a decimal > number. > > Ok > Next, I think there are macro constants if you need to print RTEMS types. > I can't find anything except pritime.h and primode.h. Perhaps someone else > can step in here for help. > > Some style issues: > > + if( should be "if (" > > + Braces should follow the denser K&R style: > if (condition) { > } else { > } > > Ok > + >> + r = pthread_join( thread_id, NULL ); >> + if( r ) >> + { >> + printf( "pthread_join : 0x%d", r ); >> + } >> + rtems_test_assert( r == 0 ); >> > > You did not set any pthread attributes so you can't be certain that the > thread you created is joinable. > >> + >> + r = pthread_getcpuclockid( thread_id, &cid ); >> + if( r != ESRCH ) >> + { >> + printf( "pthread_getcpuclockid : 0x%d", r ); >> + } >> + rtems_test_assert( r == ESRCH ); >> > > I would start with a simple functional test on a single pthread that gets > cpu clock id (should succeed). > > Then get its cpu clock and save that time. > > Use some get time API in a loop and spin (no blocking) for say 1/2 second > of wall time passage. > > Then see if the CPU time used went up 1/2 second as expected. > > Be aware that the increase in CPU time used will not be exactly 1/2 > second. It will be at least 1/2 second but there is a reasonable upper > bounds above that. > >> + >> + TEST_END(); >> + rtems_test_exit(0); >> +} >> + >> +#define CONFIGURE_APPLICATION_NEEDS_CLOCK_DRIVER >> +#define CONFIGURE_APPLICATION_NEEDS_SIMPLE_CONSOLE_DRIVER >> + >> +#define CONFIGURE_MAXIMUM_POSIX_THREADS 2 >> +#define CONFIGURE_MAXIMUM_TASKS 1 >> > > I see a need for 1 pthread in the code as written. Should stay 2 pthreads > because you need to switch to pthread initialization thread. > >> + >> +#define CONFIGURE_INITIAL_EXTENSIONS RTEMS_TEST_INITIAL_EXTENSION >> + >> +#define CONFIGURE_RTEMS_INIT_TASKS_TABLE >> > > Switch here to pthread initialization threads. And change Init task to > POSIX_Init thread. > > OK > + >> +#define CONFIGURE_INIT >> + >> +#include <rtems/confdefs.h> >> + >> diff --git >> a/testsuites/psxtests/psxgetcpuclockid01/psxgetcpuclockid01.doc >> b/testsuites/psxtests/psxgetcpuclockid01/psxgetcpuclockid01.doc >> new file mode 100644 >> index 0000000000..145f009337 >> --- /dev/null >> +++ b/testsuites/psxtests/psxgetcpuclockid01/psxgetcpuclockid01.doc >> @@ -0,0 +1,19 @@ >> +# COPYRIGHT (c) 2020 >> +# On-Line Applications Research Corporation (OAR). >> +# >> +# SPDX-License-Identifier: BSD-2-Clause >> +# >> + >> +This file describes the directives and concepts tested by this test set. >> + >> +test set name: psxgetcpuclockid 1 >> + >> +Directives: >> + Pthread_getcpuclockid() >> + >> +Concepts: >> + >> ++ This test exercises the getcpuclockid methods. >> + >> ++ For pthread_getcpuclockid it created a pthread using pthread_create >> and then finds >> +the clockid_t of that thread and then ends the thread. >> +After the thread is ended it again tried to find the clockid_t of the >> thread to check >> +for ESRCH exception. >> + >> diff --git >> a/testsuites/psxtests/psxgetcpuclockid01/psxgetcpuclockid01.scn >> b/testsuites/psxtests/psxgetcpuclockid01/psxgetcpuclockid01.scn >> new file mode 100644 >> index 0000000000..937a908fd5 >> --- /dev/null >> +++ b/testsuites/psxtests/psxgetcpuclockid01/psxgetcpuclockid01.scn >> @@ -0,0 +1,4 @@ >> +*** BEGIN OF TEST PSXGETCLOCKID 1 *** >> + >> +*** END OF TEST PSXGETCPUCLOCKID 1 *** >> + >> -- >> 2.17.1 >> >>
_______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
