NULL struct to futimes does not return current date/time on GNU/Hurd

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

NULL struct to futimes does not return current date/time on GNU/Hurd

Barry deFreese
Hello,

Please forgive me if I don't explain myself well because I am not an
uberHacker but would like to see this fixed.  Applications that pass a NULL
timeval struct to futimes (such as coreutils touch), futimes is supposed to
set the current date/time which does not work properly.

Alfred Szmidt helped me develop a patch that does work but I was hoping to
get some validation on whether this was OK or if there is a more appropiate
way to handle it (diff attached).

Please reply to me on this since I am not a subscriber to this list.

Thank you for your time!

Barry deFreese (aka bddebian)

 

futimes.diff (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: NULL struct to futimes does not return current date/time on GNU/Hurd

Thomas Schwinge-6
Hello!

On Tue, Sep 19, 2006 at 01:13:48PM -0400, Barry deFreese wrote:
> Applications that pass a NULL timeval struct to futimes (such as
> coreutils touch), futimes is supposed to set the current date/time
> which does not work properly.

I'd either suggest the following (ugly) fix:

#v+
--- futimes.c.orig 2006-09-23 01:11:56.000000000 +0200
+++ futimes.c.ugly.glibc 2006-09-23 12:42:29.000000000 +0200
@@ -28,7 +28,7 @@
 int
 __futimes (int fd, const struct timeval tvp[2])
 {
-  struct timeval timevals[2];
+  struct timeval __attribute__ ((__may_alias__)) timevals[2];
   error_t err;
 
   if (tvp == NULL)
#v-

... or rather rewrite that function:

#v+
--- futimes.c.orig 2006-09-23 01:11:56.000000000 +0200
+++ futimes.c.rewrite.glibc 2006-09-23 12:43:26.000000000 +0200
@@ -28,20 +28,20 @@
 int
 __futimes (int fd, const struct timeval tvp[2])
 {
-  struct timeval timevals[2];
   error_t err;
+  time_value_t new_atime, new_mtime;
 
   if (tvp == NULL)
-    {
       /* Setting the number of microseconds to `-1' tells the
          underlying filesystems to use the current time.  */
-      timevals[1].tv_usec = timevals[0].tv_usec = (time_t)-1;
-      tvp = timevals;
+    new_atime.microseconds = new_mtime.microseconds = -1;
+  else
+    {
+      new_atime = *(time_value_t *) &tvp[0];
+      new_mtime = *(time_value_t *) &tvp[1];
     }
 
-  err = HURD_DPORT_USE (fd, __file_utimes (port,
-   *(time_value_t *) &tvp[0],
-   *(time_value_t *) &tvp[1]));
+  err = HURD_DPORT_USE (fd, __file_utimes (port, new_atime, new_mtime));
   return err ? __hurd_dfail (fd, err) : 0;
 }
 weak_alias (__futimes, futimes)
#v-


Roland, what do you prefer?  Based on that I will then send in patches
also for the other *utimes.c files, which have similar problems, as the
code looks like.


Regards,
 Thomas

signature.asc (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: NULL struct to futimes does not return current date/time on GNU/Hurd

Alfred M. Szmidt
   Based on that I will then send in patches also for the other
   *utimes.c files, which have similar problems, as the code looks
   like.

Please show a test case that exhibits thes so called problems.
Reply | Threaded
Open this post in threaded view
|

Re: NULL struct to futimes does not return current date/time on GNU/Hurd

Samuel Thibault
In reply to this post by Thomas Schwinge-6
Hi,

Thomas Schwinge, le Sat 23 Sep 2006 12:48:56 +0200, a écrit :
> +      new_atime = *(time_value_t *) &tvp[0];
> +      new_mtime = *(time_value_t *) &tvp[1];

Mmm, this might break sometime for whatever optimizing compiler reason
too. Why not stop using bogus casts, and rather use:

Index: sysdeps/mach/hurd/futimes.c
===================================================================
RCS file: /cvs/glibc/libc/sysdeps/mach/hurd/futimes.c,v
retrieving revision 1.1
diff -u -p -r1.1 futimes.c
--- sysdeps/mach/hurd/futimes.c 27 Aug 2002 02:09:20 -0000 1.1
+++ sysdeps/mach/hurd/futimes.c 23 Sep 2006 12:29:52 -0000
@@ -28,20 +28,22 @@
 int
 __futimes (int fd, const struct timeval tvp[2])
 {
-  struct timeval timevals[2];
   error_t err;
+  time_value_t new_atime, new_mtime;
 
   if (tvp == NULL)
-    {
       /* Setting the number of microseconds to `-1' tells the
          underlying filesystems to use the current time.  */
-      timevals[1].tv_usec = timevals[0].tv_usec = (time_t)-1;
-      tvp = timevals;
+    new_atime.microseconds = new_mtime.microseconds = -1;
+  else
+    {
+      new_atime.seconds = tvp[0].tv_sec;
+      new_atime.microseconds = tvp[0].tv_usec;
+      new_mtime.seconds = tvp[1].tv_sec;
+      new_mtime.microseconds = tvp[1].tv_usec;
     }
 
-  err = HURD_DPORT_USE (fd, __file_utimes (port,
-   *(time_value_t *) &tvp[0],
-   *(time_value_t *) &tvp[1]));
+  err = HURD_DPORT_USE (fd, __file_utimes (port, new_atime, new_mtime));
   return err ? __hurd_dfail (fd, err) : 0;
 }
 weak_alias (__futimes, futimes)

which just precisely explains the compiler what we want, and nothing
more, nothing less.

Samuel