Re: Remove __ASSUME_UTIMENSAT

[Will Newton <will dot newton at linaro dot org>]

On 20 June 2014 17:14, Joseph S. Myers <joseph@codesourcery.com> wrote:
> This patch removes the __ASSUME_UTIMENSAT macro, now it can be
> unconditionally assumed to be true.
>
> This shows that the only live uses of __ASSUME_UTIMES are in utimes.c
> and they are only live for hppa.  I intend a followup patch to make
> __ASSUME_UTIMES into an hppa-specific macro (not used or defined
> outside sysdeps/unix/sysv/linux/hppa/).

Could we also switch hppa to utimensat now and just remove utimes.c?

> Tested x86_64 that the disassembly of installed shared libraries is
> unchanged by this patch.
>
> 2014-06-20  Joseph Myers  <joseph@codesourcery.com>
>
>         * sysdeps/unix/sysv/linux/kernel-features.h (__ASSUME_UTIMENSAT):
>         Remove macro.
>         * sysdeps/unix/sysv/linux/futimes.c: Do not include
>         <kernel-features.h>.
>         [__NR_utimensat && !__ASSUME_UTIMENSAT] (miss_utimensat): Remove
>         conditional variable definition.
>         (__futimes): Update comment.
>         (__futimes) [__ASSUME_UTIMENSAT]: Make code unconditional.
>         (__futimes) [!__ASSUME_UTIMENSAT]: Remove conditional code.

This looks ok to me.

> diff --git a/sysdeps/unix/sysv/linux/futimes.c b/sysdeps/unix/sysv/linux/futimes.c
> index 1b1f005..522dfe7 100644
> --- a/sysdeps/unix/sysv/linux/futimes.c
> +++ b/sysdeps/unix/sysv/linux/futimes.c
> @@ -25,19 +25,12 @@
>  #include <_itoa.h>
>  #include <fcntl.h>
>
> -#include <kernel-features.h>
> -
> -
> -#if defined __NR_utimensat && !defined __ASSUME_UTIMENSAT
> -static int miss_utimensat;
> -#endif
>
>  /* Change the access time of the file associated with FD to TVP[0] and
>     the modification time of FILE to TVP[1].
>
>     Starting with 2.6.22 the Linux kernel has the utimensat syscall which
> -   can be used to implement futimes.  Earlier kernels have no futimes()
> -   syscall so we use the /proc filesystem.  */
> +   can be used to implement futimes.  */
>  int
>  __futimes (int fd, const struct timeval tvp[2])
>  {
> @@ -56,82 +49,6 @@ __futimes (int fd, const struct timeval tvp[2])
>        TIMEVAL_TO_TIMESPEC (&tvp[1], &ts[1]);
>      }
>
> -#ifdef __ASSUME_UTIMENSAT
>    return INLINE_SYSCALL (utimensat, 4, fd, NULL, tvp ? &ts : NULL, 0);
> -#else
> -  int result;
> -# ifdef __NR_utimensat
> -  if (!__builtin_expect (miss_utimensat, 0))
> -    {
> -      result = INLINE_SYSCALL (utimensat, 4, fd, NULL, tvp ? &ts : NULL, 0);
> -      if (__builtin_expect (result, 0) != -1 || errno != ENOSYS)
> -       return result;
> -
> -      miss_utimensat = 1;
> -    }
> -# endif
> -
> -  static const char selffd[] = "/proc/self/fd/";
> -  char fname[sizeof (selffd) + 3 * sizeof (int)];
> -  fname[sizeof (fname) - 1] = '\0';
> -  char *cp = _itoa_word ((unsigned int) fd, fname + sizeof (fname) - 1, 10, 0);
> -  cp = memcpy (cp - sizeof (selffd) + 1, selffd, sizeof (selffd) - 1);
> -
> -# ifdef __NR_utimes
> -  result = INLINE_SYSCALL (utimes, 2, cp, tvp);
> -#  ifndef __ASSUME_UTIMES
> -  if (result == -1 && errno == ENOSYS)
> -#  endif
> -# endif
> -    {
> -      /* The utimes() syscall does not exist or is not available in the
> -        used kernel.  Use utime().  For this we have to convert to the
> -        data format utime() expects.  */
> -# ifndef __ASSUME_UTIMES
> -      struct utimbuf buf;
> -      struct utimbuf *times;
> -
> -      if (tvp != NULL)
> -       {
> -         times = &buf;
> -         buf.actime = tvp[0].tv_sec;
> -         buf.modtime = tvp[1].tv_sec;
> -       }
> -      else
> -       times = NULL;
> -
> -      result = INLINE_SYSCALL (utime, 2, cp, times);
> -# endif
> -    }
> -
> -  if (result == -1)
> -    /* Check for errors that result from failing to find /proc.
> -       This means we can't do futimes at all, so return ENOSYS
> -       rather than some confusing error.  */
> -    switch (errno)
> -      {
> -      case EACCES:
> -       if (tvp == NULL)  /* Could be a path problem or a file problem.  */
> -         break;
> -       /*FALLTHROUGH*/
> -      case ELOOP:
> -      case ENAMETOOLONG:
> -      case ENOTDIR:
> -       __set_errno (ENOSYS);
> -       break;
> -
> -      case ENOENT:
> -       /* Validate the file descriptor by letting fcntl set errno to
> -          EBADF if it's bogus.  Otherwise it's a /proc issue.  */
> -# if !defined __NR_fcntl && defined __NR_fcntl64
> -#  define __NR_fcntl __NR_fcntl64
> -# endif
> -       if (INLINE_SYSCALL (fcntl, 3, fd, F_GETFD, 0) != -1)
> -         __set_errno (ENOSYS);
> -       break;
> -      }
> -
> -  return result;
> -#endif
>  }
>  weak_alias (__futimes, futimes)
> diff --git a/sysdeps/unix/sysv/linux/kernel-features.h b/sysdeps/unix/sysv/linux/kernel-features.h
> index 026f89f..88bde0d 100644
> --- a/sysdeps/unix/sysv/linux/kernel-features.h
> +++ b/sysdeps/unix/sysv/linux/kernel-features.h
> @@ -105,10 +105,6 @@
>  # define __ASSUME_FUTEX_LOCK_PI        1
>  #endif
>
> -/* Support for utimensat syscall was added in 2.6.22, on SH
> -   only after 2.6.22-rc1.  */
> -#define __ASSUME_UTIMENSAT     1
> -
>  /* Support for private futexes was added in 2.6.22.  */
>  #define __ASSUME_PRIVATE_FUTEX 1
>
>
> --
> Joseph S. Myers
> joseph@codesourcery.com



-- 
Will Newton
Toolchain Working Group, Linaro