On Wed, May 12, 2021 at 11:33 AM Gedare Bloom <ged...@rtems.org> wrote:
>
> On Wed, May 12, 2021 at 7:33 AM Ryan Long <ryan.l...@oarcorp.com> wrote:
> >
> > Created futimens.c and utimensat.c to add support for the POSIX
> > methods futimens() and utimensat().
> >
> > utime() and utimes() are considered obsolote by POSIX, but RTEMS
> > will continue to support them.
> >
> > Closes #4396
> > ---
> > cpukit/Makefile.am | 2 +
> > cpukit/include/rtems/libio_.h | 64 ++++++++-
> > cpukit/include/rtems/score/timespec.h | 16 ++-
> > cpukit/libcsupport/src/futimens.c | 87 ++++++++++++
> > cpukit/libcsupport/src/utimensat.c | 224
> > +++++++++++++++++++++++++++++++
> > cpukit/score/src/timespecisnonnegative.c | 35 +++++
> > spec/build/cpukit/librtemscpu.yml | 3 +
> > 7 files changed, 426 insertions(+), 5 deletions(-)
> > create mode 100644 cpukit/libcsupport/src/futimens.c
> > create mode 100644 cpukit/libcsupport/src/utimensat.c
> > create mode 100644 cpukit/score/src/timespecisnonnegative.c
> >
> > diff --git a/cpukit/Makefile.am b/cpukit/Makefile.am
> > index b0df610..29b4207 100644
> > --- a/cpukit/Makefile.am
> > +++ b/cpukit/Makefile.am
> > @@ -262,6 +262,8 @@ librtemscpu_a_SOURCES += libcsupport/src/unmount.c
> > librtemscpu_a_SOURCES += libcsupport/src/__usrenv.c
> > librtemscpu_a_SOURCES += libcsupport/src/utime.c
> > librtemscpu_a_SOURCES += libcsupport/src/utimes.c
> > +librtemscpu_a_SOURCES += libcsupport/src/futimens.c
> > +librtemscpu_a_SOURCES += libcsupport/src/utimensat.c
> > librtemscpu_a_SOURCES += libcsupport/src/utsname.c
> > librtemscpu_a_SOURCES += libcsupport/src/vprintk.c
> > librtemscpu_a_SOURCES += libcsupport/src/write.c
> > diff --git a/cpukit/include/rtems/libio_.h b/cpukit/include/rtems/libio_.h
> > index e9eb462..b7455ef 100644
> > --- a/cpukit/include/rtems/libio_.h
> > +++ b/cpukit/include/rtems/libio_.h
> > @@ -2,13 +2,12 @@
> > * @file
> > *
> > * @brief LibIO Internal Interface
> > - *
> > + *
> > * This file is the libio internal interface.
> > */
> >
> > /*
> > - * COPYRIGHT (c) 1989-2011.
> > - * On-Line Applications Research Corporation (OAR).
> > + * COPYRIGHT (C) 1989, 2021 On-Line Applications Research Corporation
> > (OAR).
> > *
> > * Modifications to support reference counting in the file system are
> > * Copyright (c) 2012 embedded brains GmbH.
> > @@ -30,6 +29,7 @@
> > #include <rtems/libio.h>
> > #include <rtems/seterr.h>
> > #include <rtems/score/assert.h>
> > +#include <rtems/score/timespec.h>
> >
> > #ifdef __cplusplus
> > extern "C" {
> > @@ -357,6 +357,64 @@ static inline void rtems_filesystem_instance_unlock(
> > (*mt_entry->ops->unlock_h)( mt_entry );
> > }
> >
> > +/**
> > + * @brief Checks the tv_nsec member of a timespec struct
> > + *
> > + * This function is used with utimensat() and futimens() only. This ensures
> > + * that the value in the tv_nsec member is equal to either UTIME_NOW,
> > + * UTIME_OMIT, or a value greater-than or equal to zero and less than a
> > + * billion.
> > + *
> > + * @param[in] time The timespec struct to be validated
> > + *
> > + * @retval Returns true if tv_nsec member is a valid value, otherwise
> > false.
> > + */
> > +bool rtems_filesystem_utime_tv_nsec_valid( struct timespec time );
> > +
> > +/**
> > + * @brief Determines if the process has write permissions to a file
> I think it is more specific than that. Also, missing period.
>
> > + *
> > + * The timespec at index 0 is the access time, the timespec at index 1 is
> > the
> > + * modification time.
> Should this note be on the @param[in] times below?
>
> > + *
> > + * This function is only used with utimensat() and futimens(). This checks
> > + * whether the process has write access to a file, and checks for EACCES
> > + * and EPERM errors depending on what values are in @times and if the
> > process
> > + * has write permissions to the file.
> > + *
> > + * @param[in] currentloc The current location to a file
> > + * @param[in] times The timespec instance that will update the timestamps
> update? nothing about this function seems to be an update?
>
> > + *
> > + * @retval Returns 0 if the process has write permissions, otherwise -1.
> > + */
> > +int rtems_filesystem_utime_check_permissions(
> > + const rtems_filesystem_location_info_t *currentloc,
> > + const struct timespec times[2]
> > +);
> > +
> > +/**
> > + * @brief Checks @times and fills @new_times with the time to be written
> > + *
> > + * For each of the arguments, the timespec at index 0 is the access time,
> > and
> > + * the timespec at index 1 is the modification time.
> > + *
> > + * This function is only used with utimensat() and futimens(). @times
> > contains
> > + * the constant values passed into utimensat/futimens. @new_times contains
> > the
> > + * values that will be written to the file. These values depend on @times.
> > If
> > + * @times is NULL, or either of its elements' tv_nsec members are
> > UTIME_NOW,
> > + * the current elapsed time in nanoseconds will be saved in the
> > corresponding
> > + * location in @new_times.
> > + *
> > + * @param[in] times The timespecs to be checked
> > + * @param[in,out] new_times The timespecs containing the time to be written
> I don't think new_times is an 'in' param?
>
> > + *
> > + * @retval Returns 0 if @times is valid, otherwise -1.
> > + */
> > +int rtems_filesystem_utime_check_times(
> this function name is strange. A "check" normally doesn't have side
> effects? This seems to be updating the new_times.
>
> > + const struct timespec times[2],
> > + struct timespec new_times[2]
> > +);
> > +
> > /*
> > * File Descriptor Routine Prototypes
> > */
> > diff --git a/cpukit/include/rtems/score/timespec.h
> > b/cpukit/include/rtems/score/timespec.h
> > index 314d804..3671067 100644
> > --- a/cpukit/include/rtems/score/timespec.h
> > +++ b/cpukit/include/rtems/score/timespec.h
> > @@ -8,8 +8,7 @@
> > */
> >
> > /*
> > - * COPYRIGHT (c) 1989-2009.
> > - * On-Line Applications Research Corporation (OAR).
> > + * COPYRIGHT (C) 1989, 2021 On-Line Applications Research Corporation
> > (OAR).
> > *
> > * The license and distribution terms for this file may be
> > * found in the file LICENSE in this distribution or at
You can update this license while you're at it.
> > @@ -106,6 +105,19 @@ uint64_t _Timespec_Get_as_nanoseconds(
> > );
> >
> > /**
> > + * @brief Checks the tv_sec member of a timespec struct
> clarify this brief.
>
> > + *
> > + * @param[in] time is the timespec instance to validate.
> > + *
> > + * Ensures that the values in @a time are non-negative.
> This would be a better brief, except it should be "checks if" rather
> than "ensures that"
>
> > + *
> > + * @retval Returns true if the tv_sec member is a valid value, otherwise
> > false.
> this is not the right way to write retval, the value should come first
> * @retval true The tv_sec member is a valid value.
> * @retval false The tv_sec member is not a valid value.
> This problem exists elsewhere/throughout.
>
> If our doxygen guidelines are not clear, we should update them.
>
> > + */
> > +bool _Timespec_Is_non_negative(
> > + const struct timespec *time
> > +);
> > +
> > +/**
> > * @brief Checks if timespec is valid.
> > *
> > * This method determines the validity of a timespec.
> > diff --git a/cpukit/libcsupport/src/futimens.c
> > b/cpukit/libcsupport/src/futimens.c
> > new file mode 100644
> > index 0000000..44ec03d
> > --- /dev/null
> > +++ b/cpukit/libcsupport/src/futimens.c
> > @@ -0,0 +1,87 @@
> > +/* SPDX-License-Identifier: BSD-2-Clause */
> > +
> > +/**
> > + * @file
> > + *
> > + * @ingroup libcsupport
> > + *
> > + * @brief Set file access and modification times based on file descriptor
> > in
> > + * nanoseconds.
> > + */
> > +
> > +/*
> > + * COPYRIGHT (C) 2021 On-Line Applications Research Corporation (OAR).
> > + *
> > + * 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 <sys/stat.h>
> > +#include <rtems/libio_.h>
> > +
> > +#include <fcntl.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +
> > +/**
> > + *
> > https://pubs.opengroup.org/onlinepubs/9699919799.2008edition/functions/futimens.html
> > + *
> > + * Set file access and modification times
> > + */
> > +int futimens(
> > + int fd,
> > + const struct timespec times[2]
> > +)
> > +{
> > + int rv;
> > + rtems_libio_t *iop;
> > + struct timespec new_times[2];
> > + const rtems_filesystem_location_info_t *currentloc = NULL;
> > +
> > + LIBIO_GET_IOP_WITH_ACCESS( fd, iop, LIBIO_FLAGS_READ, EBADF );
> > +
> > + currentloc = &iop->pathinfo;
> > +
> > + rv = rtems_filesystem_utime_check_times( times, new_times );
> > + if ( rv != 0 ) {
> > + rtems_libio_iop_drop( iop );
> > + return rv;
> > + }
> > +
> > + rv = rtems_filesystem_utime_check_permissions( currentloc, times );
> > + if ( rv != 0 ) {
> > + rtems_libio_iop_drop( iop );
> > + return rv;
> > + }
> > +
> > + rv = (*currentloc->mt_entry->ops->utimens_h)(
> I don't think this utimens_h exists yet. How did you compile RTEMS
> successfully with this patch applied, before applying Patches 2 and 3?
>
> > + currentloc,
> > + new_times
> > + );
> > +
> > + rtems_libio_iop_drop( iop );
> > +
> > + return rv;
> > +}
> > diff --git a/cpukit/libcsupport/src/utimensat.c
> > b/cpukit/libcsupport/src/utimensat.c
> > new file mode 100644
> > index 0000000..59ba369
> > --- /dev/null
> > +++ b/cpukit/libcsupport/src/utimensat.c
> > @@ -0,0 +1,224 @@
> > +/* SPDX-License-Identifier: BSD-2-Clause */
> > +
> > +/**
> > + * @file
> > + *
> > + * @ingroup libcsupport
> > + *
> > + * @brief Set file access and modification times in nanoseconds.
> > + */
> > +
> > +/*
> > + * COPYRIGHT (C) 2021 On-Line Applications Research Corporation (OAR).
> > + *
> > + * 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 <rtems/libio_.h>
> > +
> > +#include <fcntl.h>
> > +#include <string.h>
> > +
> > +/*
> > + * Make sure that tv_nsec is either UTIME_NOW, UTIME_OMIT, a value
> > + * greater than zero, or a value less-than a billion.
> > + *
> > + * These guidelines come from the description of the EINVAL errors on
> > + * https://pubs.opengroup.org/onlinepubs/9699919799/functions/futimens.html
> > + */
> > +bool rtems_filesystem_utime_tv_nsec_valid(struct timespec time)
> > +{
> > + if ( time.tv_nsec == UTIME_NOW ) {
> > + return true;
> > + }
> > +
> > + if ( time.tv_nsec == UTIME_OMIT ) {
> > + return true;
> > + }
> > +
> > + if ( time.tv_nsec < 0 ) {
> > + return false;
> > + }
> > +
> > + if ( time.tv_nsec > 999999999 ) {
> Elsewhere we have instead used
> nsec >= TOD_NANOSECONDS_PER_SECOND
> That would be preferred here also.
>
> > + return false;
> > + }
> > +
> > + return true;
> > +}
> > +
> > +/* Determine whether the access and modified timestamps can be updated */
> > +int rtems_filesystem_utime_check_permissions(
> > + const rtems_filesystem_location_info_t * currentloc,
> > + const struct timespec times[2]
> > +)
> > +{
> > + struct stat st = {};
> > + int rv;
> > + bool write_access;
> > +
> > + rv = (*currentloc->handlers->fstat_h)( currentloc, &st );
> > + if ( rv != 0 ) {
> > + rtems_set_errno_and_return_minus_one( ENOENT );
> > + }
> > +
> > + write_access = rtems_filesystem_check_access(
> > + RTEMS_FS_PERMS_WRITE,
> > + st.st_mode,
> > + st.st_uid,
> > + st.st_gid
> > + );
> > +
> > + /*
> > + * The logic for the EACCES error is an inverted subset of the EPERM
> > + * conditional according to the POSIX standard.
> > + */
> > + if ( (times == NULL) ||
> Did you test the times == NULL case?
>
> > + ( (times[0].tv_nsec == UTIME_NOW) && (times[1].tv_nsec == UTIME_NOW)
> > )) {
> > + if ( !write_access ) {
> > + rtems_set_errno_and_return_minus_one( EACCES );
> > + }
> > + } else {
> > + if ( times[0].tv_nsec != UTIME_OMIT || times[1].tv_nsec != UTIME_OMIT
> > ) {
> What happens here if times==NULL?
>
NVM on this and the question on testing, I got my logic backward here.
> > + if ( !write_access ) {
> > + rtems_set_errno_and_return_minus_one( EPERM );
> > + }
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +/*
> > + * Determine if the current time needs to be gotten, and then check
> > + * whether the values in times are valid.
> > + */
> > +int rtems_filesystem_utime_check_times(
> > + const struct timespec times[2],
> > + struct timespec new_times[2]
> > +)
> > +{
> > + bool got_time = false;
> > + struct timespec now;
> > +
> > + /*
> > + * If times is NULL, it's equivalent to adding UTIME_NOW in both time
> > + * elements
> > + */
> > + if ( times == NULL ) {
> > + _Timespec_Set( &new_times[0], 0, UTIME_NOW );
> > + _Timespec_Set( &new_times[1], 0, UTIME_NOW );
> > + } else {
> > + new_times[0] = times[0];
> > + new_times[1] = times[1];
> > + }
> > +
> > + if ( new_times[0].tv_nsec == UTIME_NOW ) {
> > + clock_gettime( CLOCK_REALTIME, &now );
> > + new_times[0] = now;
> > + got_time = true;
> > + }
> > +
> > + if ( new_times[1].tv_nsec == UTIME_NOW ) {
> > + if ( !got_time ) {
> > + clock_gettime( CLOCK_REALTIME, &now );
> > + }
> > + new_times[1] = now;
> > + }
> > +
> > + if ( !_Timespec_Is_non_negative( &new_times[0] ) ) {
> > + rtems_set_errno_and_return_minus_one( EINVAL );
> > + }
> > +
> > + if ( !_Timespec_Is_non_negative( &new_times[1] ) ) {
> > + rtems_set_errno_and_return_minus_one( EINVAL );
> > + }
> > +
> > + if ( !rtems_filesystem_utime_tv_nsec_valid( new_times[0] ) ) {
> > + rtems_set_errno_and_return_minus_one( EINVAL );
> > + }
> > +
> > + if ( !rtems_filesystem_utime_tv_nsec_valid( new_times[1] ) ) {
> > + rtems_set_errno_and_return_minus_one( EINVAL );
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + *
> > https://pubs.opengroup.org/onlinepubs/9699919799.2008edition/functions/futimens.html
> > + *
> > + * Set file access and modification times
> > + */
> > +int utimensat(
> > + int fd,
> > + const char *path,
> > + const struct timespec times[2],
> > + int flag
> > +)
> > +{
> > + int rv = 0;
> > + rtems_filesystem_eval_path_context_t ctx;
> > + int eval_flags = RTEMS_FS_FOLLOW_LINK;
> > + const rtems_filesystem_location_info_t *currentloc = NULL;
> > + struct timespec new_times[2];
> > +
> > + /*
> > + * RTEMS does not currently support operating on a real file descriptor
> > + */
> > + if ( fd != AT_FDCWD ) {
> > + rtems_set_errno_and_return_minus_one( ENOSYS );
> > + }
> > +
> > + /*
> > + * RTEMS does not currently support AT_SYMLINK_NOFOLLOW
> > + */
> > + if ( flag != 0 ) {
> > + rtems_set_errno_and_return_minus_one( ENOSYS );
> > + }
> > +
> > + rv = rtems_filesystem_utime_check_times( times, new_times );
> > + if ( rv != 0 ) {
> > + return rv;
> > + }
> > +
> > + currentloc = rtems_filesystem_eval_path_start( &ctx, path, eval_flags );
> > +
> > + rv = rtems_filesystem_utime_check_permissions( currentloc, times );
> indent wrong here
>
> > + if ( rv != 0 ) {
> > + rtems_filesystem_eval_path_cleanup( &ctx );
> > + return rv;
> > + }
> > +
> > + rv = (*currentloc->mt_entry->ops->utimens_h)(
> > + currentloc,
> > + new_times
> > + );
> > +
> > + rtems_filesystem_eval_path_cleanup( &ctx );
> > +
> > + return rv;
> > +}
> > diff --git a/cpukit/score/src/timespecisnonnegative.c
> > b/cpukit/score/src/timespecisnonnegative.c
> > new file mode 100644
> > index 0000000..080e961
> > --- /dev/null
> > +++ b/cpukit/score/src/timespecisnonnegative.c
> > @@ -0,0 +1,35 @@
>
> spdx
>
> > +/**
> > + * @file
> > + *
> > + * @ingroup RTEMSScoreTimespec
> > + *
> > + * @brief This source file contains the implementation of
> > + * _Timespec_Is_non_negative().
> > + */
> > +
> > +/*
> > + * COPYRIGHT (C) 2021 On-Line Applications Research Corporation (OAR).
> > + *
> > + * The license and distribution terms for this file may be
> > + * found in the file LICENSE in this distribution or at
> > + * http://www.rtems.org/license/LICENSE.
> 2-bsd?
>
> > + */
> > +
> > +#ifdef HAVE_CONFIG_H
> > +#include "config.h"
> > +#endif
> > +
> > +#include <rtems/score/timespec.h>
> > +
> > +bool _Timespec_Is_non_negative(
> > + const struct timespec *time
> > +)
> > +{
> > + if ( time->tv_sec < 0 )
> > + return false;
> > +
> > + if ( time->tv_nsec < 0 )
> > + return false;
> > +
> > + return true;
> > +}
> > diff --git a/spec/build/cpukit/librtemscpu.yml
> > b/spec/build/cpukit/librtemscpu.yml
> > index a3a9ee4..7f36ef7 100644
> > --- a/spec/build/cpukit/librtemscpu.yml
> > +++ b/spec/build/cpukit/librtemscpu.yml
> > @@ -761,6 +761,8 @@ source:
> > - cpukit/libcsupport/src/unmount.c
> > - cpukit/libcsupport/src/utime.c
> > - cpukit/libcsupport/src/utimes.c
> > +- cpukit/libcsupport/src/futimens.c
> > +- cpukit/libcsupport/src/utimensat.c
> > - cpukit/libcsupport/src/utsname.c
> > - cpukit/libcsupport/src/vprintk.c
> > - cpukit/libcsupport/src/write.c
> > @@ -1558,6 +1560,7 @@ source:
> > - cpukit/score/src/threadyield.c
> > - cpukit/score/src/timespecaddto.c
> > - cpukit/score/src/timespecdivide.c
> > +- cpukit/score/src/timespecisnonnegative.c
> > - cpukit/score/src/timespecdividebyinteger.c
> > - cpukit/score/src/timespecfromticks.c
> > - cpukit/score/src/timespecgetasnanoseconds.c
> > --
> > 1.8.3.1
> >
> > _______________________________________________
> > devel mailing list
> > devel@rtems.org
> > http://lists.rtems.org/mailman/listinfo/devel
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
