[PATCH] Confstr Patches

Joel Sherrill joel at rtems.org
Fri Sep 11 21:15:38 UTC 2020


This looks close to me but I see some minor issues. I have tried to
annotate them.

The spacing ones could be spaces vs tabs. Not sure.

On Thu, Sep 10, 2020 at 3:48 PM Eshan dhawan <eshandhawan51 at gmail.com>
wrote:

> Adds Confstr file To RTEMS with test
>
> Signed-off-by: Eshan dhawan <eshandhawan51 at gmail.com>
> ---
>  cpukit/Makefile.am                            |   1 +
>  cpukit/posix/src/confstr.c                    | 105 +++++++++++++++
>  testsuites/psxtests/Makefile.am               |   9 ++
>  testsuites/psxtests/configure.ac              |   1 +
>  testsuites/psxtests/psxconfstr/init.c         | 125 ++++++++++++++++++
>  testsuites/psxtests/psxconfstr/psxconfstr.doc |  17 +++
>  testsuites/psxtests/psxconfstr/psxconfstr.scn |   4 +
>  7 files changed, 262 insertions(+)
>  create mode 100644 cpukit/posix/src/confstr.c
>  create mode 100644 testsuites/psxtests/psxconfstr/init.c
>  create mode 100644 testsuites/psxtests/psxconfstr/psxconfstr.doc
>  create mode 100644 testsuites/psxtests/psxconfstr/psxconfstr.scn
>
> diff --git a/cpukit/Makefile.am b/cpukit/Makefile.am
> index e5009e53c9..1c9e4697f1 100644
> --- a/cpukit/Makefile.am
> +++ b/cpukit/Makefile.am
> @@ -509,6 +509,7 @@ librtemscpu_a_SOURCES += posix/src/condsignalsupp.c
>  librtemscpu_a_SOURCES += posix/src/condtimedwait.c
>  librtemscpu_a_SOURCES += posix/src/condwait.c
>  librtemscpu_a_SOURCES += posix/src/condwaitsupp.c
> +librtemscpu_a_SOURCES += posix/src/confstr.c
>  librtemscpu_a_SOURCES += posix/src/_execve.c
>  librtemscpu_a_SOURCES += posix/src/fork.c
>  librtemscpu_a_SOURCES += posix/src/key.c
> diff --git a/cpukit/posix/src/confstr.c b/cpukit/posix/src/confstr.c
> new file mode 100644
> index 0000000000..71041db462
> --- /dev/null
> +++ b/cpukit/posix/src/confstr.c
> @@ -0,0 +1,105 @@
> +/*-
> + * SPDX-License-Identifier: BSD-3-Clause
> + *
> + * Copyright (c) 1993
> + *     The Regents of the University of California.  All rights reserved.
> + *
> + * 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.
> + * 3. Neither the name of the University nor the names of its contributors
> + *    may be used to endorse or promote products derived from this
> software
> + *    without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE REGENTS 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 REGENTS 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.
> + */
> +
> +#include <sys/cdefs.h>
> +__SCCSID("@(#)confstr.c        8.1 (Berkeley) 6/4/93");
> +__FBSDID("$FreeBSD$");
>

Probably worth adding a comment above that the FreeBSD version
was reduced to what made sense on RTEMS.

+
> +#include <sys/param.h>
> +
> +#include <errno.h>
> +#include <limits.h>
> +#include <paths.h>
> +#include <string.h>
> +#include <unistd.h>
>
> If you do that, you won't need all these headers.


> +size_t
> +confstr(int name, char *buf, size_t len)
> +{
> +       const char *p;
> +       const char UPE[] = "unsupported programming environment";
> +
> +       switch (name) {
> +#ifndef __rtems__
> +       case _CS_PATH:
> +               p = _PATH_STDPATH;
> +               goto docopy;
> +#endif
>

It looks like the body of the other cases were gutted from the freebsd
code. Why keep an ifndef RTEMS here? This is just going to produce
an error for an unknown name via the default case. If we are going to
delete all non-FreeBSD code from this and make it a more RTEMS
specific implementation, you might as well just do this:

        case _CS_PATH:
              errno = EINVAL;
              return (0);

That keeps the order like the FreeBSD without ifdefs.


> +               /*
> +                * POSIX/SUS ``Programming Environments'' stuff
> +                *
> +                * We don't support more than one programming environment
> +                * on any platform (yet), so we just return the empty
> +                * string for the environment we are compiled for,
> +                * and the string "unsupported programming environment"
> +                * for anything else.  (The Standard says that if these
> +                * values are used on a system which does not support
> +                * this environment -- determined via sysconf() -- then
> +                * the value we return is unspecified.  So, we return
> +                * something which will cause obvious breakage.)
> +                */
> +       case _CS_POSIX_V6_ILP32_OFF32_CFLAGS:
> +       case _CS_POSIX_V6_ILP32_OFF32_LDFLAGS:
> +       case _CS_POSIX_V6_ILP32_OFF32_LIBS:
> +       case _CS_POSIX_V6_LPBIG_OFFBIG_CFLAGS:
> +       case _CS_POSIX_V6_LPBIG_OFFBIG_LDFLAGS:
> +       case _CS_POSIX_V6_LPBIG_OFFBIG_LIBS:
> +
> +               p = UPE;
> +               goto docopy;
> +
> +       case _CS_POSIX_V6_ILP32_OFFBIG_CFLAGS:
> +       case _CS_POSIX_V6_ILP32_OFFBIG_LDFLAGS:
> +       case _CS_POSIX_V6_ILP32_OFFBIG_LIBS:
> +
> +                       p = UPE;
> +               goto docopy;
>


Spacing.

> +
> +       case _CS_POSIX_V6_LP64_OFF64_CFLAGS:
> +       case _CS_POSIX_V6_LP64_OFF64_LDFLAGS:
> +       case _CS_POSIX_V6_LP64_OFF64_LIBS:
> +
> +                       p = UPE;
> +               goto docopy;
>

Spacing.

> +
> +
> +docopy:
> +               if (len != 0 && buf != NULL)
> +                       strlcpy(buf, p, len);
> +               return (strlen(p) + 1);
> +
> +       default:
> +               errno = EINVAL;
> +               return (0);
> +       }
> +       /* NOTREACHED */
> +}
> diff --git a/testsuites/psxtests/Makefile.am
> b/testsuites/psxtests/Makefile.am
> index aae5fe764a..f0910330a9 100755
> --- a/testsuites/psxtests/Makefile.am
> +++ b/testsuites/psxtests/Makefile.am
> @@ -358,6 +358,15 @@ psxconfig01_CPPFLAGS = $(AM_CPPFLAGS)
> $(TEST_FLAGS_psxconfig01) \
>         $(support_includes) -I$(top_srcdir)/include
>  endif
>
> +if TEST_psxgetcpuclockid01
> +psx_tests += psxconfstr
> +psx_screens += psxconfstr/psxconfstr.scn
> +psx_docs += psxconfstr/psxconfstr.doc
> +psxconfstr_SOURCES = psxconfstr/init.c
> +psxconfstr_CPPFLAGS = $(AM_CPPFLAGS) $(TEST_FLAGS_psxgetcpuclockid01) \
> +       $(support_includes)
> +endif
> +
>  if TEST_psxdevctl01
>  psx_tests += psxdevctl01
>  psx_screens += psxdevctl01/psxdevctl01.scn
> diff --git a/testsuites/psxtests/configure.ac b/testsuites/psxtests/
> configure.ac
> index c509086abc..96c937cb4d 100644
> --- a/testsuites/psxtests/configure.ac
> +++ b/testsuites/psxtests/configure.ac
> @@ -80,6 +80,7 @@ RTEMS_TEST_CHECK([psxconcurrency01])
>  RTEMS_TEST_CHECK([psxcond01])
>  RTEMS_TEST_CHECK([psxcond02])
>  RTEMS_TEST_CHECK([psxconfig01])
> +RTEMS_TEST_CHECK([psxconfstr])
>  RTEMS_TEST_CHECK([psxdevctl01])
>  RTEMS_TEST_CHECK([psxeintr_join])
>  RTEMS_TEST_CHECK([psxenosys])
> diff --git a/testsuites/psxtests/psxconfstr/init.c
> b/testsuites/psxtests/psxconfstr/init.c
> new file mode 100644
> index 0000000000..7219470559
> --- /dev/null
> +++ b/testsuites/psxtests/psxconfstr/init.c
> @@ -0,0 +1,125 @@
> +/*
> + *  @file
> + *  @brief Test suite for confstr method
> + */
> +
> +/*
> + * SPDX-License-Identifier: BSD-2-Clause
> + *
> + * Copyright (C) 2020 Eshan Dhawan, Sebastian Huber
> + *
> + * 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 <stdio.h>
> +#include <string.h>
> +#include <rtems.h>
> +#include <rtems/test.h>
> +#include <unistd.h>
> +#include <rtems/bspIo.h>
> +
> +#include "tmacros.h"
> +#include "test_support.h"
> +
> +const char rtems_test_name[] = "PSXCONFSTR";
> +
> +/* Forward declaration to avoid warnings */
> +rtems_task Init( rtems_task_argument ignored );
> +
> +static char buffer[512];
> +
> +static const T_action actions[] = {
> +  T_report_hash_sha256,
> +  T_check_task_context,
> +  T_check_file_descriptors,
> +  T_check_rtems_barriers,
> +  T_check_rtems_extensions,
> +  T_check_rtems_message_queues,
> +  T_check_rtems_partitions,
> +  T_check_rtems_periods,
> +  T_check_rtems_regions,
> +  T_check_rtems_semaphores,
> +  T_check_rtems_tasks,
> +  T_check_rtems_timers,
> +  T_check_posix_keys
> +};
>
> Is this a remnant of a copied test? Is it used?


> +static const T_config config = {
> +  .name = "psxconfstr",
> +  .buf = buffer,
> +  .putchar = rtems_put_char,
> +  .buf_size = sizeof(buffer),
> +  .verbosity = T_VERBOSE,
> +  .now = T_now_clock,
> +  .action_count = T_ARRAY_SIZE(actions),
> +  .actions = actions
> +};
> +
> +/* init test function begins */
> +T_TEST_CASE(confstr)
> +{
> +
> +int r;
> +  char * buf ;
> +  const char UPE[] = "unsupported programming environment";
> +  size_t len1;
> +  len1 = strlen(UPE) + 1;
> +  r = confstr(_CS_PATH, buf, sizeof(buf));
> +
> +  T_quiet_psx_success(r);
> +  r = confstr(_CS_POSIX_V6_ILP32_OFFBIG_CFLAGS, buf, sizeof(buf));
> +  T_quiet_eq_int(r, len1);
> +
> +}
>
> You should exercise every known good value for a name. Plus a randomly bad
one.


> +rtems_task Init(rtems_task_argument ignored)
> +{
> +  int exit_code;
> +
> +  TEST_BEGIN();
> +
> +  T_register();
> +  exit_code = T_main(&config);
> +  if (exit_code == 0) {
> +    TEST_END();
> +  }
> +
> + rtems_test_exit(exit_code);
> +
> +}
> +#define CONFIGURE_APPLICATION_DOES_NOT_NEED_CLOCK_DRIVER
> +#define CONFIGURE_APPLICATION_NEEDS_CONSOLE_DRIVER
> +
> +#define CONFIGURE_MAXIMUM_TASKS 1
> +
> +#define CONFIGURE_INITIAL_EXTENSIONS RTEMS_TEST_INITIAL_EXTENSION
> +
> +#define CONFIGURE_RTEMS_INIT_TASKS_TABLE
> +#define CONFIGURE_INIT
> +
> +#include <rtems/confdefs.h>
> +/* end of file */
> diff --git a/testsuites/psxtests/psxconfstr/psxconfstr.doc
> b/testsuites/psxtests/psxconfstr/psxconfstr.doc
> new file mode 100644
> index 0000000000..e353f74ae7
> --- /dev/null
> +++ b/testsuites/psxtests/psxconfstr/psxconfstr.doc
> @@ -0,0 +1,17 @@
> +#  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:  psxconfstr
> +
> +Directives:
> +  Pthread_getcpuclockid()
>

Not right.


> +
> +Concepts:
> +
> ++ This test exercises the confstr method
> +
> diff --git a/testsuites/psxtests/psxconfstr/psxconfstr.scn
> b/testsuites/psxtests/psxconfstr/psxconfstr.scn
> new file mode 100644
> index 0000000000..705a08d51b
> --- /dev/null
> +++ b/testsuites/psxtests/psxconfstr/psxconfstr.scn
> @@ -0,0 +1,4 @@
> +*** BEGIN OF TEST PSXGETCPUCLOCKID 1 ***
> +
> +*** END OF TEST PSXGETCPUCLOCKID 1 ***
> +
>

I'm betting this isn't right either. :)

--joel


> --
> 2.17.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20200911/e2a7c7ff/attachment-0001.html>


More information about the devel mailing list