Re: [Qemu-devel] [PATCH] Fix utimensat (aka unbreak cp -a)

[Riku Voipio <riku.voipio@iki.fi>]

On Tue, Apr 21, 2009 at 10:24:03AM +0200, Arnaud Patard wrote:
> Don't use the glibc function for utimensat because glibc returns -EINVAL
> if the path is null which is a different behaviour with the syscall.
> path can be null because internally the glibc is using utimensat with
> path null (for instance, see __futimes in
> sysdeps/unix/sysv/linux/futimes.c in glibc tree).

Soo.. glibc uses utimensat to implement futimens, but doesn't allow
applications to use utimensat in the same way. What a mess.

But if we are to go towards using libc calls, your patch is a step
backwards. In case pathname is null, we can use futimens.


commit 0f34ff059fb9ad6e8c8fa5161d9d17265286bb62
Author: Riku Voipio <riku.voipio@iki.fi>
Date:   Tue Apr 21 15:01:51 2009 +0300

    linux-user: fix utimensat when used as futimens
    
    The glibc function for utimensat glibc returns -EINVAL when the path is null
    which is a different behaviour with the syscall.
    
    path can be null because internally the glibc is using utimensat with
    path null when implmenting futimens. If path is null, call futimes
    instead.

diff --git a/configure b/configure
index 08df436..fc980a4 100755
--- a/configure
+++ b/configure
@@ -1208,6 +1208,25 @@ EOF
   fi
 fi
 
+# check if utimensat and futimens are supported
+utimens=no
+cat > $TMPC << EOF
+#define _ATFILE_SOURCE
+#define _GNU_SOURCE
+#include <stddef.h>
+#include <fcntl.h>
+
+int main(void)
+{
+    utimensat(AT_FDCWD, "foo", NULL, 0);
+    futimens(0, NULL);
+    return 0;
+}
+EOF
+if $cc $ARCH_CFLAGS -o $TMPE $TMPC 2> /dev/null ; then
+  utimens=yes
+fi
+
 # Check if tools are available to build documentation.
 if [ -x "`which texi2html 2>/dev/null`" ] && \
    [ -x "`which pod2man 2>/dev/null`" ]; then
@@ -1604,6 +1623,9 @@ fi
 if test "$atfile" = "yes" ; then
   echo "#define CONFIG_ATFILE 1" >> $config_h
 fi
+if test "$utimens" = "yes" ; then
+  echo "#define CONFIG_UTIMENSAT 1" >> $config_h
+fi
 if test "$inotify" = "yes" ; then
   echo "#define CONFIG_INOTIFY 1" >> $config_h
 fi
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 408ccc6..4dad5c1 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -406,13 +406,6 @@ static int sys_unlinkat(int dirfd, const char *pathname, int flags)
   return (unlinkat(dirfd, pathname, flags));
 }
 #endif
-#ifdef TARGET_NR_utimensat
-static int sys_utimensat(int dirfd, const char *pathname,
-    const struct timespec times[2], int flags)
-{
-  return (utimensat(dirfd, pathname, times, flags));
-}
-#endif
 #else /* !CONFIG_ATFILE */
 
 /*
@@ -476,12 +469,24 @@ _syscall3(int,sys_symlinkat,const char *,oldpath,
 #if defined(TARGET_NR_unlinkat) && defined(__NR_unlinkat)
 _syscall3(int,sys_unlinkat,int,dirfd,const char *,pathname,int,flags)
 #endif
+
+#endif /* CONFIG_ATFILE */
+
+#ifdef CONFIG_UTIMENSAT
+static int sys_utimensat(int dirfd, const char *pathname,
+    const struct timespec times[2], int flags)
+{
+    if (pathname == NULL)
+        return futimens(dirfd, times);
+    else
+        return utimensat(dirfd, pathname, times, flags);
+}
+#else
 #if defined(TARGET_NR_utimensat) && defined(__NR_utimensat)
 _syscall4(int,sys_utimensat,int,dirfd,const char *,pathname,
           const struct timespec *,tsp,int,flags)
 #endif
-
-#endif /* CONFIG_ATFILE */
+#endif /* CONFIG_UTIMENSAT  */
 
 #ifdef CONFIG_INOTIFY
 #include <sys/inotify.h>