[PATCH v1 1/5] libcsupport: Added futimens() and utimensat()
Ryan Long
ryan.long at oarcorp.com
Tue May 4 16:04:05 UTC 2021
-----Original Message-----
From: Sebastian Huber <sebastian.huber at embedded-brains.de>
Sent: Tuesday, May 4, 2021 12:04 AM
To: Ryan Long <ryan.long at oarcorp.com>; devel at rtems.org
Subject: Re: [PATCH v1 1/5] libcsupport: Added futimens() and utimensat()
On 03/05/2021 15:40, Ryan Long 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 | 61 +++++++++-
> cpukit/libcsupport/src/futimens.c | 91 +++++++++++++++
> cpukit/libcsupport/src/utimensat.c | 229 +++++++++++++++++++++++++++++++++++++
> spec/build/cpukit/librtemscpu.yml | 2 +
> 5 files changed, 384 insertions(+), 1 deletion(-)
> create mode 100644 cpukit/libcsupport/src/futimens.c
> create mode 100644 cpukit/libcsupport/src/utimensat.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..fc7a1e4 100644
> --- a/cpukit/include/rtems/libio_.h
> +++ b/cpukit/include/rtems/libio_.h
> @@ -7,7 +7,7 @@
> */
>
> /*
> - * COPYRIGHT (c) 1989-2011.
> + * COPYRIGHT (c) 1989-2011, 2021.
> * On-Line Applications Research Corporation (OAR).
> *
> * Modifications to support reference counting in the file system
> are @@ -357,6 +357,65 @@ static inline void rtems_filesystem_instance_unlock(
> (*mt_entry->ops->unlock_h)( mt_entry );
> }
>
> +/* Prototypes for functions used between utimensat() and futimens()
> +*/
> +
> +/**
> + * @brief Checks the tv_sec member of a timespec struct
> + *
> + * @param[in] time The timespec struct to be validated
> + *
> + * Ensures that the value in the tv_sec member is non-negative.
> + *
> + * @retval true If the tv_sec member is a valid value
> + * @retval false If the tv_sec member is not a valid value */ bool
> +rtems_filesystem_utime_tv_sec_valid( struct timespec time );
> +
> +/**
> + * @brief Checks the tv_nsec member of a timespec struct
> + *
> + * 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 true If the tv_nsec member is a valid value
> + * @retval false If the tv_nsec member is not a valid value */ bool
> +rtems_filesystem_utime_tv_nsec_valid( struct timespec time );
Are these two functions ever used individually? Do they modify the time?
Maybe we should add a standard phase for boolean type returns:
@return Returns true, if the X is valid, otherwise false.
[Ryan Long] These functions are only used in rtems_filesystem_utime_check_times(). They don't modify the time, they just ensure that the members of the timespec struct are valid values. Should the arguments for those functions be changed to be a const since they're not being modified? And sure, I can consolidate the @retval statements like that.
> +
> +/**
> + * @brief Determines if the process has write permissions to a file
> + *
> + * Checks that the process has the same userID as the file and
> +whether the
> + * file has write permissions.
> + *
> + * @param[in] currentloc The current location to a file
> + * @param[in] fstat_h The file handler of @currentloc
Why is this an extra parameter?
[Ryan Long] This was Joel's suggestion, but I didn't think about just using the currentloc argument. I'll change that in V2.
> + *
> + * @retval 0 If the process has write permissions
> + * @retval -1 If the process does not have write permissions
Why 0 and -1, what about errno? Is it set by the function or not?
[Ryan Long] It's 0 and -1 because it uses the rtems_set_errno_and_return_minus_one() macro.
> + */
> +int rtems_filesystem_utime_check_permissions(
> + const rtems_filesystem_location_info_t *currentloc,
> + rtems_filesystem_fstat_t fstat_h
> +);
> +
> +/**
> + * @brief Checks @times and updates @new_times
> + *
> + * @param[in] times The timespec struct to be checked
> + * @param[in,out] new_times The timespec struct to contain the
> +updated time
> + *
> + * @retval 0 If the time was able to be updated
> + * @retval -1 If the time was not able to be updated */ int
> +rtems_filesystem_utime_check_times(
> + const struct timespec times[2],
> + struct timespec new_times[2]
> +);
> +
> /*
> * File Descriptor Routine Prototypes
> */
> diff --git a/cpukit/libcsupport/src/futimens.c
> b/cpukit/libcsupport/src/futimens.c
> new file mode 100644
> index 0000000..33ae469
> --- /dev/null
> +++ b/cpukit/libcsupport/src/futimens.c
> @@ -0,0 +1,91 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +
> +/**
> + * @file
> + *
> + * @ingroup libcsupport
> + *
> + * @brief Set file access and modification times based on file
> +descriptor in
> + * nanoseconds.
> + */
> +
> +/*
> + * COPYRIGHT (c) 1989-2009, 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/function
> +s/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_permissions(
> + currentloc,
> + (*currentloc->handlers->fstat_h)
> + );
> + if( rv == -1 ) {
Which coding style should this file and the other new files have?
[...]
> +}
> +
> +/* Determine whether the access and modified timestamps can be
> +updated */ int rtems_filesystem_utime_check_permissions(
> + const rtems_filesystem_location_info_t * currentloc,
> + rtems_filesystem_fstat_t fstat_h
> +)
> +{
> + struct stat st;
> + volatile int rv;
volatile?
[Ryan Long] Forgot to take this out after debugging. Will be removed in V2.
> + bool write_access;
> +
> + memset( &st, 0, sizeof(st) );
> +
> + rv = (*fstat_h)( currentloc, &st ); if( rv != 0 ) {
> + rtems_set_errno_and_return_minus_one( ENOENT ); }
> +
> + /*
> + * Check that the user ID and the user ID of the file match
> + */
> + if( geteuid() != st.st_uid ) {
> + rtems_set_errno_and_return_minus_one( EACCES );
> + }
Is this really what POSIX specified?
[Ryan Long] Yes, on https://pubs.opengroup.org/onlinepubs/9699919799.2008edition/functions/futimens.html they said return this if 'the effective user ID of the process does not match the owner of the file and write access is denied.'
> +
> + write_access = rtems_filesystem_check_access(
> + RTEMS_FS_PERMS_WRITE,
> + st.st_mode,
> + st.st_uid,
> + st.st_gid
> + );
> + if( !write_access ) {
> + rtems_set_errno_and_return_minus_one( EACCES );
> + }
> +
> + 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 );
> + }
> +
> + currentloc = rtems_filesystem_eval_path_start( &ctx, path, eval_flags );
> +
> + rv = rtems_filesystem_utime_check_permissions(
> + currentloc,
> + (*currentloc->handlers->fstat_h)
> + );
> + if( rv == -1 ) {
Better use the success code, for example rv != 0.
> + rtems_filesystem_eval_path_cleanup( &ctx );
> + return rv;
> + }
> +
> + rv = rtems_filesystem_utime_check_times( times, new_times);
> + if( rv == -1 ) {
> + 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/spec/build/cpukit/librtemscpu.yml b/spec/build/cpukit/librtemscpu.yml
> index a3a9ee4..9639051 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
--
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber at embedded-brains.de
phone: +49-89-18 94 741 - 16
fax: +49-89-18 94 741 - 08
Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/
More information about the devel
mailing list