[PATCH] Change file_utimes RPC to use a struct timespec and update the servers to use UTIME_NOW and UTIME_OMIT.

[Flávio Cruz]

From: Flávio Cruz <***@gmail.com>

Hello

These two patches allow the glibc and the hurd servers to handle UTIME_OMIT and UTIME_NOW in futimens.
The file_utimes RPC now uses a struct timespec instead of a timeval.

---
console-client/trans.c | 8 ++------
console/console.c | 8 ++------
ext2fs/pager.c | 3 +++
ftpfs/netfs.c | 8 ++------
hostmux/node.c | 8 ++------
hurd/fs.defs | 4 ++--
libdiskfs/file-utimes.c | 20 +++++++++++--------
libnetfs/file-utimes.c | 40 +++++++++++++++++++------------------
libnetfs/init-init.c | 6 ++++++
libnetfs/netfs.h | 2 +-
libnetfs/priv.h | 2 ++
libtreefs/s-file.c | 2 +-
libtreefs/treefs-s-hooks.h | 2 +-
libtrivfs/file-utimes.c | 2 +-
libtrivfs/times.c | 24 +++++++++++-----------
nfs/nfs.c | 50 ++++++++++++++++++++++++++++++++++++----------
nfs/ops.c | 14 ++++---------
nfsd/ops.c | 46 +++++++++++++++++++++---------------------
trans/fakeroot.c | 29 +++++++++++++--------------
usermux/node.c | 8 ++------
20 files changed, 153 insertions(+), 133 deletions(-)

diff --git a/console-client/trans.c b/console-client/trans.c
index 224229e..4c78e46 100644
--- a/console-client/trans.c
+++ b/console-client/trans.c
@@ -186,14 +186,10 @@ netfs_attempt_utimes (struct iouser *cred, struct node *np,
if (! err)
{
if (mtime)
- np->nn_stat.st_mtim = *mtime;
- else
- flags |= TOUCH_MTIME;
+ np->nn_stat.st_mtim = *mtime;

if (atime)
- np->nn_stat.st_atim = *atime;
- else
- flags |= TOUCH_ATIME;
+ np->nn_stat.st_atim = *atime;

fshelp_touch (&np->nn_stat, flags, console_maptime);
}
diff --git a/console/console.c b/console/console.c
index 57ae813..1be1112 100644
--- a/console/console.c
+++ b/console/console.c
@@ -506,14 +506,10 @@ netfs_attempt_utimes (struct iouser *cred, struct node *node,
if (! err)
{
if (mtime)
- node->nn_stat.st_mtim = *mtime;
- else
- flags |= TOUCH_MTIME;
+ node->nn_stat.st_mtim = *mtime;

if (atime)
- node->nn_stat.st_atim = *atime;
- else
- flags |= TOUCH_ATIME;
+ node->nn_stat.st_atim = *atime;

fshelp_touch (&node->nn_stat, flags, console_maptime);
}
diff --git a/ext2fs/pager.c b/ext2fs/pager.c
index b56c923..4f56743 100644
--- a/ext2fs/pager.c
+++ b/ext2fs/pager.c
@@ -1220,6 +1220,9 @@ create_disk_pager (void)
err = pager_start_workers (file_pager_bucket);
if (err)
ext2_panic ("can't create libpager worker threads: %s", strerror (err));
+#ifdef STAT
+
+#endif
}

/* Call this to create a FILE_DATA pager and return a send right.
diff --git a/ftpfs/netfs.c b/ftpfs/netfs.c
index cf5d907..b0c80db 100644
--- a/ftpfs/netfs.c
+++ b/ftpfs/netfs.c
@@ -77,14 +77,10 @@ netfs_attempt_utimes (struct iouser *cred, struct node *node,
if (! err)
{
if (atime)
- node->nn_stat.st_atim = *atime;
- else
- flags |= TOUCH_ATIME;
+ node->nn_stat.st_atim = *atime;

if (mtime)
- node->nn_stat.st_mtim = *mtime;
- else
- flags |= TOUCH_MTIME;
+ node->nn_stat.st_mtim = *mtime;

fshelp_touch (&node->nn_stat, flags, ftpfs_maptime);
}
diff --git a/hostmux/node.c b/hostmux/node.c
index 7167300..0724d35 100644
--- a/hostmux/node.c
+++ b/hostmux/node.c
@@ -79,14 +79,10 @@ netfs_attempt_utimes (struct iouser *cred, struct node *node,
if (! err)
{
if (mtime)
- node->nn_stat.st_mtim = *mtime;
- else
- flags |= TOUCH_MTIME;
+ node->nn_stat.st_mtim = *mtime;

if (atime)
- node->nn_stat.st_atim = *atime;
- else
- flags |= TOUCH_ATIME;
+ node->nn_stat.st_atim = *atime;

fshelp_touch (&node->nn_stat, flags, hostmux_maptime);
}
diff --git a/hurd/fs.defs b/hurd/fs.defs
index a4a48cc..1514a84 100644
--- a/hurd/fs.defs
+++ b/hurd/fs.defs
@@ -99,8 +99,8 @@ routine file_chflags (
routine file_utimes (
utimes_file: file_t;
RPT
- new_atime: time_value_t;
- new_mtime: time_value_t);
+ new_atime: timespec_t;
+ new_mtime: timespec_t);

/* Change the size of the file. If the size increases, new blocks are
zero-filled. After successful return, it is safe to reference mapped
diff --git a/libdiskfs/file-utimes.c b/libdiskfs/file-utimes.c
index 39fac50..d525e81 100644
--- a/libdiskfs/file-utimes.c
+++ b/libdiskfs/file-utimes.c
@@ -21,28 +21,32 @@
/* Implement file_utimes as described in <hurd/fs.defs>. */
kern_return_t
diskfs_S_file_utimes (struct protid *cred,
- time_value_t atime,
- time_value_t mtime)
+ struct timespec atime,
+ struct timespec mtime)
{
CHANGE_NODE_FIELD (cred,
({
if (!(err = fshelp_isowner (&np->dn_stat, cred->user)))
{
- if (atime.microseconds == -1)
+ if (atime.tv_nsec == UTIME_NOW)
np->dn_set_atime = 1;
+ else if(atime.tv_nsec == UTIME_OMIT)
+ np->dn_set_atime = 0;
else
{
- np->dn_stat.st_atim.tv_sec = atime.seconds;
- np->dn_stat.st_atim.tv_nsec = atime.microseconds * 1000;
+ np->dn_stat.st_atim.tv_sec = atime.tv_sec;
+ np->dn_stat.st_atim.tv_nsec = atime.tv_nsec;
np->dn_set_atime = 0;
}

- if (mtime.microseconds == -1)
+ if (mtime.tv_nsec == UTIME_NOW)
np->dn_set_mtime = 1;
+ else if(mtime.tv_nsec == UTIME_OMIT)
+ np->dn_set_mtime = 0;
else
{
- np->dn_stat.st_mtim.tv_sec = mtime.seconds;
- np->dn_stat.st_mtim.tv_nsec = mtime.microseconds * 1000;
+ np->dn_stat.st_mtim.tv_sec = mtime.tv_sec;
+ np->dn_stat.st_mtim.tv_nsec = mtime.tv_nsec;
np->dn_set_mtime = 0;
}

diff --git a/libnetfs/file-utimes.c b/libnetfs/file-utimes.c
index 1915609..2c49cfa 100644
--- a/libnetfs/file-utimes.c
+++ b/libnetfs/file-utimes.c
@@ -23,31 +23,33 @@

error_t
netfs_S_file_utimes (struct protid *user,
- time_value_t atimein,
- time_value_t mtimein)
+ struct timespec atimein,
+ struct timespec mtimein)
{
- struct timespec atime, mtime;
error_t err;
+ struct timeval t;

- if (atimein.microseconds != -1)
- {
- atime.tv_sec = atimein.seconds;
- atime.tv_nsec = atimein.microseconds * 1000;
- }
-
- if (mtimein.microseconds != -1)
- {
- mtime.tv_sec = mtimein.seconds;
- mtime.tv_nsec = mtimein.microseconds * 1000;
- }
-
if (!user)
return EOPNOTSUPP;
-
+
+ if (atimein.tv_nsec == UTIME_NOW || mtimein.tv_nsec == UTIME_NOW)
+ maptime_read (netfs_mtime, &t);
+
+ if (atimein.tv_nsec == UTIME_NOW)
+ {
+ atimein.tv_sec = t.tv_sec;
+ atimein.tv_nsec = t.tv_usec * 1000;
+ }
+ if (mtimein.tv_nsec == UTIME_NOW)
+ {
+ mtimein.tv_nsec = t.tv_sec;
+ mtimein.tv_nsec = t.tv_usec * 1000;
+ }
+
pthread_mutex_lock (&user->po->np->lock);
- err = netfs_attempt_utimes (user->user, user->po->np,
- atimein.microseconds != -1 ? &atime : 0,
- mtimein.microseconds != -1 ? &mtime : 0);
+ err = netfs_attempt_utimes (user->user, user->po->np,
+ (atimein.tv_nsec == UTIME_OMIT) ? 0 : &atimein,
+ (mtimein.tv_nsec == UTIME_OMIT) ? 0 : &mtimein);
pthread_mutex_unlock (&user->po->np->lock);
return err;
}
diff --git a/libnetfs/init-init.c b/libnetfs/init-init.c
index a088ad5..4b749f5 100644
--- a/libnetfs/init-init.c
+++ b/libnetfs/init-init.c
@@ -32,11 +32,17 @@ struct port_class *netfs_protid_class = 0;
struct port_class *netfs_control_class = 0;
auth_t netfs_auth_server_port = 0;
mach_port_t netfs_fsys_identity;
+volatile struct mapped_time_value *netfs_mtime;


void
netfs_init ()
{
+ error_t err;
+ err = maptime_map (0, 0, &netfs_mtime);
+ if (err)
+ error (2, err, "mapping time");
+
netfs_protid_class = ports_create_class (netfs_release_protid, 0);
netfs_control_class = ports_create_class (0, 0);
netfs_port_bucket = ports_create_bucket ();
diff --git a/libnetfs/netfs.h b/libnetfs/netfs.h
index fbe2c60..3774070 100644
--- a/libnetfs/netfs.h
+++ b/libnetfs/netfs.h
@@ -173,7 +173,7 @@ error_t netfs_attempt_chflags (struct iouser *cred, struct node *np,
/* The user must define this function. This should attempt a utimes
call for the user specified by CRED on locked node NP, to change
the atime to ATIME and the mtime to MTIME. If ATIME or MTIME is
- null, then set to the current time. */
+ null, then do not change it. */
error_t netfs_attempt_utimes (struct iouser *cred, struct node *np,
struct timespec *atime, struct timespec *mtime);

diff --git a/libnetfs/priv.h b/libnetfs/priv.h
index 3c5bcd4..3871da8 100644
--- a/libnetfs/priv.h
+++ b/libnetfs/priv.h
@@ -25,6 +25,8 @@

#include "netfs.h"

+volatile struct mapped_time_value *netfs_mtime;
+
static inline struct protid * __attribute__ ((unused))
begin_using_protid_port (file_t port)
{
diff --git a/libtreefs/s-file.c b/libtreefs/s-file.c
index c24d645..120258f 100644
--- a/libtreefs/s-file.c
+++ b/libtreefs/s-file.c
@@ -227,7 +227,7 @@ treefs_S_file_set_size (struct treefs_protid *cred, off_t size)

error_t
treefs_S_file_utimes (struct treefs_protid *cred,
- time_value_t atime, time_value_t mtime)
+ struct timespec atime, struct timespec mtime)
{
if (!cred)
return EOPNOTSUPP;
diff --git a/libtreefs/treefs-s-hooks.h b/libtreefs/treefs-s-hooks.h
index 2ea9e7a..d637459 100644
--- a/libtreefs/treefs-s-hooks.h
+++ b/libtreefs/treefs-s-hooks.h
@@ -53,7 +53,7 @@ DHH(s_file_chmod, error_t, mode_t)
DHH(s_file_chflags, error_t, int)
#define treefs_s_file_chflags(h, args...) \
_TREEFS_CHH(h, S_FILE_CHFLAGS, s_file_chflags , ##args)
-DHH(s_file_utimes, error_t, time_value_t, time_value_t)
+DHH(s_file_utimes, error_t, struct timespec, struct timespec)
#define treefs_s_file_utimes(h, args...) \
_TREEFS_CHH(h, S_FILE_UTIMES, s_file_utimes , ##args)
DHH(s_file_truncate, error_t, off_t)
diff --git a/libtrivfs/file-utimes.c b/libtrivfs/file-utimes.c
index 827c055..92b906d 100644
--- a/libtrivfs/file-utimes.c
+++ b/libtrivfs/file-utimes.c
@@ -21,7 +21,7 @@
kern_return_t
trivfs_S_file_utimes (struct trivfs_protid *cred,
mach_port_t reply, mach_msg_type_name_t reply_type,
- time_value_t atime, time_value_t mtime)
+ struct timespec atime, struct timespec mtime)
{
return cred ? file_utimes (cred->realnode, atime, mtime) : EOPNOTSUPP;
}
diff --git a/libtrivfs/times.c b/libtrivfs/times.c
index 5f08cb1..3bc07c3 100644
--- a/libtrivfs/times.c
+++ b/libtrivfs/times.c
@@ -21,13 +21,13 @@ error_t
trivfs_set_atime (struct trivfs_control *cntl)
{
struct stat st;
- time_value_t atime;
- time_value_t mtime;
+ struct timespec mtime;
+ struct timespec atime;

- io_stat (cntl->underlying, &st);
- mtime.seconds = st.st_mtim.tv_sec;
- mtime.microseconds = st.st_mtim.tv_nsec / 1000;
- atime.microseconds = -1;
+ mtime.tv_sec =0;
+ mtime.tv_nsec = UTIME_OMIT;
+ atime.tv_sec = 0;
+ atime.tv_nsec = UTIME_NOW;
file_utimes (cntl->underlying, atime, mtime);
return 0;
}
@@ -36,13 +36,13 @@ error_t
trivfs_set_mtime (struct trivfs_control *cntl)
{
struct stat st;
- time_value_t atime;
- time_value_t mtime;
+ struct timespec atime;
+ struct timespec mtime;

- io_stat (cntl->underlying, &st);
- atime.seconds = st.st_atim.tv_sec;
- atime.microseconds = st.st_atim.tv_nsec / 1000;
- mtime.microseconds = -1;
+ atime.tv_sec = 0;
+ atime.tv_nsec = UTIME_OMIT;
+ mtime.tv_sec = 0;
+ mtime.tv_nsec = UTIME_NOW;
file_utimes (cntl->underlying, atime, mtime);
return 0;
}
diff --git a/nfs/nfs.c b/nfs/nfs.c
index 4916df6..e743d40 100644
--- a/nfs/nfs.c
+++ b/nfs/nfs.c
@@ -273,10 +273,26 @@ xdr_encode_sattr_times (int *p, struct timespec *atime, struct timespec *mtime)
*(p++) = -1; /* uid */
*(p++) = -1; /* gid */
*(p++) = -1; /* size */
- *(p++) = htonl (atime->tv_sec);
- *(p++) = htonl (atime->tv_nsec / 1000);
- *(p++) = htonl (mtime->tv_sec);
- *(p++) = htonl (mtime->tv_nsec / 1000);
+ if (atime)
+ {
+ *(p++) = htonl (atime->tv_sec);
+ *(p++) = htonl (atime->tv_nsec / 1000);
+ }
+ else
+ {
+ *(p++) = -1; /* no atime */
+ *(p++) = -1;
+ }
+ if (mtime)
+ {
+ *(p++) = htonl (mtime->tv_sec);
+ *(p++) = htonl (mtime->tv_nsec / 1000);
+ }
+ else
+ {
+ *(p++) = -1; /* no mtime */
+ *(p++) = -1;
+ }
}
else
{
@@ -284,12 +300,26 @@ xdr_encode_sattr_times (int *p, struct timespec *atime, struct timespec *mtime)
*(p++) = 0; /* no uid */
*(p++) = 0; /* no gid */
*(p++) = 0; /* no size */
- *(p++) = htonl (SET_TO_CLIENT_TIME); /* atime */
- *(p++) = htonl (atime->tv_sec);
- *(p++) = htonl (atime->tv_nsec);
- *(p++) = htonl (SET_TO_CLIENT_TIME); /* mtime */
- *(p++) = htonl (mtime->tv_sec);
- *(p++) = htonl (mtime->tv_nsec);
+ if (atime)
+ {
+ *(p++) = htonl (SET_TO_CLIENT_TIME); /* atime */
+ *(p++) = htonl (atime->tv_sec);
+ *(p++) = htonl (atime->tv_nsec);
+ }
+ else
+ {
+ *(p++) = DONT_CHANGE; /* no atime */
+ }
+ if (mtime)
+ {
+ *(p++) = htonl (SET_TO_CLIENT_TIME); /* mtime */
+ *(p++) = htonl (mtime->tv_sec);
+ *(p++) = htonl (mtime->tv_nsec);
+ }
+ else
+ {
+ *(p++) = DONT_CHANGE; /* no mtime */
+ }
}
return p;
}
diff --git a/nfs/ops.c b/nfs/ops.c
index a4d6ac7..faae658 100644
--- a/nfs/ops.c
+++ b/nfs/ops.c
@@ -298,17 +298,13 @@ netfs_attempt_utimes (struct iouser *cred, struct node *np,
int *p;
void *rpcbuf;
error_t err;
- struct timeval tv;
struct timespec current;

+ if (!atime && !mtime)
+ return 0; /* nothing to update */
+
/* XXX For version 3 we can actually do this right, but we don't
just yet. */
- if (!atime || !mtime)
- {
- maptime_read (mapped_time, &tv);
- current.tv_sec = tv.tv_sec;
- current.tv_nsec = tv.tv_usec * 1000;
- }

p = nfs_initialize_rpc (NFSPROC_SETATTR (protocol_version),
cred, 0, &rpcbuf, np, -1);
@@ -316,9 +312,7 @@ netfs_attempt_utimes (struct iouser *cred, struct node *np,
return errno;

p = xdr_encode_fhandle (p, &np->nn->handle);
- p = xdr_encode_sattr_times (p,
- atime ?: &current,
- mtime ?: &current);
+ p = xdr_encode_sattr_times (p, atime, mtime);
if (protocol_version == 3)
*(p++) = 0; /* guard check == 0 */

diff --git a/nfsd/ops.c b/nfsd/ops.c
index 6e2cbb1..a83859e 100644
--- a/nfsd/ops.c
+++ b/nfsd/ops.c
@@ -63,7 +63,7 @@ complete_setattr (mach_port_t port,
{
uid_t uid, gid;
off_t size;
- time_value_t atime, mtime;
+ struct timespec atime, mtime;
struct stat st;
error_t err;

@@ -91,33 +91,33 @@ complete_setattr (mach_port_t port,
if (err)
return err;

- atime.seconds = ntohl (*p);
+ atime.tv_sec = ntohl (*p);
p++;
- atime.microseconds = ntohl (*p);
+ atime.tv_nsec = ntohl (*p) * 1000;
p++;
- mtime.seconds = ntohl (*p);
+ mtime.tv_sec = ntohl (*p);
p++;
- mtime.microseconds = ntohl (*p);
+ mtime.tv_nsec = ntohl (*p) * 1000;
p++;

- if (atime.seconds != -1 && atime.microseconds == -1)
- atime.microseconds = 0;
- if (mtime.seconds != -1 && mtime.microseconds == -1)
- mtime.microseconds = 0;
-
- if (atime.seconds == -1)
- atime.seconds = st.st_atim.tv_sec;
- if (atime.microseconds == -1)
- atime.microseconds = st.st_atim.tv_nsec / 1000;
- if (mtime.seconds == -1)
- mtime.seconds = st.st_mtim.tv_sec;
- if (mtime.microseconds == -1)
- mtime.microseconds = st.st_mtim.tv_nsec / 1000;
-
- if (atime.seconds != st.st_atim.tv_sec
- || atime.microseconds != st.st_atim.tv_nsec / 1000
- || mtime.seconds != st.st_mtim.tv_sec
- || mtime.microseconds != st.st_mtim.tv_nsec / 1000)
+ if (atime.tv_sec != -1 && atime.tv_nsec == -1)
+ atime.tv_nsec = 0;
+ if (mtime.tv_sec != -1 && mtime.tv_nsec == -1)
+ mtime.tv_nsec = 0;
+
+ if (atime.tv_nsec == -1)
+ atime.tv_sec = st.st_atim.tv_sec;
+ if (atime.tv_nsec == -1)
+ atime.tv_nsec = st.st_atim.tv_nsec;
+ if (mtime.tv_sec == -1)
+ mtime.tv_sec = st.st_mtim.tv_sec;
+ if (mtime.tv_nsec == -1)
+ mtime.tv_nsec = st.st_mtim.tv_nsec;
+
+ if (atime.tv_sec != st.st_atim.tv_sec
+ || atime.tv_nsec != st.st_atim.tv_nsec
+ || mtime.tv_sec != st.st_mtim.tv_sec
+ || mtime.tv_nsec != st.st_mtim.tv_nsec)
err = file_utimes (port, atime, mtime);

return err;
diff --git a/trans/fakeroot.c b/trans/fakeroot.c
index 76fc901..8cd7119 100644
--- a/trans/fakeroot.c
+++ b/trans/fakeroot.c
@@ -618,26 +618,25 @@ error_t
netfs_attempt_utimes (struct iouser *cred, struct node *np,
struct timespec *atime, struct timespec *mtime)
{
- union tv
- {
- struct timeval tv;
- time_value_t tvt;
- };
- union tv a, m;
+ struct timespec tatime, tmtime;
+
if (atime)
- {
- TIMESPEC_TO_TIMEVAL (&a.tv, atime);
- }
+ tatime = *atime;
else
- a.tv.tv_sec = a.tv.tv_usec = -1;
+ {
+ tatime.tv_sec = 0;
+ tatime.tv_nsec = UTIME_OMIT;
+ }
+
if (mtime)
- {
- TIMESPEC_TO_TIMEVAL (&m.tv, mtime);
- }
+ tmtime = *mtime;
else
- m.tv.tv_sec = m.tv.tv_usec = -1;
+ {
+ tmtime.tv_sec = 0;
+ tmtime.tv_nsec = UTIME_OMIT;
+ }

- return file_utimes (netfs_node_netnode (np)->file, a.tvt, m.tvt);
+ return file_utimes (netfs_node_netnode (np)->file, tatime, tmtime);
}

error_t
diff --git a/usermux/node.c b/usermux/node.c
index 2341714..66bf79b 100644
--- a/usermux/node.c
+++ b/usermux/node.c
@@ -81,14 +81,10 @@ netfs_attempt_utimes (struct iouser *cred, struct node *node,
if (! err)
{
if (mtime)
- node->nn_stat.st_mtim = *mtime;
- else
- flags |= TOUCH_MTIME;
+ node->nn_stat.st_mtim = *mtime;

if (atime)
- node->nn_stat.st_atim = *atime;
- else
- flags |= TOUCH_ATIME;
+ node->nn_stat.st_atim = *atime;

fshelp_touch (&node->nn_stat, flags, usermux_maptime);
}

[Samuel Thibault]

Hello,

Thanks for working on this!

Post by Flávio Cruz
These two patches allow the glibc and the hurd servers to handle UTIME_OMIT and UTIME_NOW in futimens.
The file_utimes RPC now uses a struct timespec instead of a timeval.

Well, we can't really change RPCs on the fly like this, otherwise you
can't upgrade running translators first or libc first. You need to
introduce a new RPC with the new interface, and in libc cope with
translators which don't know the new RPC yet by reverting to the old
RPC.

Samuel

[Samuel Thibault]

Post by Samuel Thibault

Post by Flávio Cruz
These two patches allow the glibc and the hurd servers to handle UTIME_OMIT and UTIME_NOW in futimens.
The file_utimes RPC now uses a struct timespec instead of a timeval.

Well, we can't really change RPCs on the fly like this, otherwise you
can't upgrade running translators first or libc first. You need to
introduce a new RPC with the new interface, and in libc cope with
translators which don't know the new RPC yet by reverting to the old
RPC.

(And translators should keep serving both RPCs, for applications running
with old libc).

Samuel

[Flávio Cruz]

Post by Samuel Thibault
Hello,
Thanks for working on this!

Post by Flávio Cruz
These two patches allow the glibc and the hurd servers to handle

UTIME_OMIT and UTIME_NOW in futimens.

Post by Flávio Cruz
The file_utimes RPC now uses a struct timespec instead of a timeval.

Well, we can't really change RPCs on the fly like this, otherwise you
can't upgrade running translators first or libc first. You need to
introduce a new RPC with the new interface, and in libc cope with
translators which don't know the new RPC yet by reverting to the old
RPC.

That makes sense. I tested the patches by cross-compiling the system and I
did not thought about compatibility issues.

I'm attaching the modified patches (still trying to figure how to reply
using git send-email). I added a new file_utimens RPC to handle timespecs
in fs.defs and changed the glibc and translator implementations accordingly.

Note that I also added a two macros to convert between timespecs and
time_value_t in libshouldbeinlibc/timefmt.h (one was taken from the exec
server). Not sure if this is the best place.

Flavio

Post by Samuel Thibault
Samuel

[Justus Winter]

Quoting Flávio Cruz (2015-08-31 17:16:08)

I'm attaching the modified patches (still trying to figure how to reply using
git send-email).

git send-email --in-reply-to=MSGIDTOREPLYTOHERE --compose 000*

Cheers,
Justus

[Samuel Thibault]

Post by Flávio Cruz
Note that I also added a two macros to convert between timespecs and
time_value_t in libshouldbeinlibc/timefmt.h (one was taken from the exec
server). Not sure if this is the best place.

It seems a bit odd to put it in "timefmt" which means time
format, indeed. Perhaps we should rather add them to gnumach's
mach/time_value.h?

Post by Flávio Cruz
--- a/sysdeps/mach/hurd/futimens.c
+++ b/sysdeps/mach/hurd/futimens.c
{
- atime.seconds = tsp[0].tv_sec;
- atime.microseconds = tsp[0].tv_nsec / 1000;
- mtime.seconds = tsp[1].tv_sec;
- mtime.microseconds = tsp[1].tv_nsec / 1000;
+ atime.tv_sec = tsp[0].tv_sec;
+ atime.tv_nsec = tsp[0].tv_nsec;
+ mtime.tv_sec = tsp[1].tv_sec;
+ mtime.tv_nsec = tsp[1].tv_nsec;

Mmm, better just atime=tsp[0]; mtime=tsp[1];?

Post by Flávio Cruz
--- a/sysdeps/mach/hurd/futimes.c
+++ b/sysdeps/mach/hurd/futimes.c
@@ -27,24 +27,44 @@
+ if(tvp == NULL)

Take care of formatting.

Post by Flávio Cruz
--- a/sysdeps/mach/hurd/lutimes.c
+++ b/sysdeps/mach/hurd/lutimes.c
+ if(tvp == NULL)

ditto, and probably other places as well.

Post by Flávio Cruz
+/* Implement file_utimens as described in <hurd/fs.defs>. */
+kern_return_t
+diskfs_S_file_utimens (struct protid *cred,
+ struct timespec atime,
+ struct timespec mtime)
+{
CHANGE_NODE_FIELD (cred,
- ({
- if (!(err = fshelp_isowner (&np->dn_stat, cred->user)))
+ ({
+ if (!(err = fshelp_isowner (&np->dn_stat, cred->user)))

Please avoid reformatting indentation even if the existing is odd, it
both makes reading patches clumsier, and makes git annotate to find
changes harder to use.

Post by Flávio Cruz
+ if (atime.tv_nsec == UTIME_NOW)
+ np->dn_set_atime = 1;
+ else if (atime.tv_nsec == UTIME_OMIT)
+ np->dn_set_atime = 0;

I don't think you want to set dn_set_atime to 0 in these places. If there was a
previous utime call which used UTIME_NOW for instance, we don't want to
interfere with that. I think in the UTIME_OMIT case we should just do
nothing.

Post by Flávio Cruz
diff --git a/libnetfs/file-utimes.c b/libnetfs/file-utimes.c
index 1915609..2bf22f7 100644
--- a/libnetfs/file-utimes.c
+++ b/libnetfs/file-utimes.c
+ if (atimein.tv_nsec == UTIME_NOW || mtimein.tv_nsec == UTIME_NOW)

{

Post by Flávio Cruz
+ maptime_read (netfs_mtime, &t);
+
+ if (atimein.tv_nsec == UTIME_NOW)
+ TIMEVAL_TO_TIMESPEC (&t, &atimein);
+ if (mtimein.tv_nsec == UTIME_NOW)
+ TIMEVAL_TO_TIMESPEC (&t, &mtimein);

}

which will be a bit more efficient, and avoid dumb compilers to emit
warnings about t being undefined.

Post by Flávio Cruz
--- a/libtreefs/treefs-s-hooks.h
+++ b/libtreefs/treefs-s-hooks.h
@@ -53,7 +53,7 @@ DHH(s_file_chmod, error_t, mode_t)
DHH(s_file_chflags, error_t, int)
#define treefs_s_file_chflags(h, args...) \
_TREEFS_CHH(h, S_FILE_CHFLAGS, s_file_chflags , ##args)
-DHH(s_file_utimes, error_t, time_value_t, time_value_t)
+DHH(s_file_utimens, error_t, struct timespec, struct timespec)
#define treefs_s_file_utimes(h, args...) \
_TREEFS_CHH(h, S_FILE_UTIMES, s_file_utimes , ##args)

This looks odd: don't we need both utimes and utimens?

Post by Flávio Cruz
diff --git a/libtrivfs/times.c b/libtrivfs/times.c
index 5f08cb1..4c5a0e4 100644
--- a/libtrivfs/times.c
+++ b/libtrivfs/times.c
error_t
trivfs_set_atime (struct trivfs_control *cntl)
{
- io_stat (cntl->underlying, &st);
+ mtime.tv_sec = 0;
+ mtime.tv_nsec = UTIME_OMIT;

that's a nice side effect :)

Post by Flávio Cruz
--- a/nfs/nfs.c
+++ b/nfs/nfs.c
+ else
+ *(p++) = DONT_CHANGE; /* no atime */

also nice :)

Samuel

[Flávio Cruz]

Hi Samuel

Post by Samuel Thibault

Post by Flávio Cruz
Note that I also added a two macros to convert between timespecs and
time_value_t in libshouldbeinlibc/timefmt.h (one was taken from the exec
server). Not sure if this is the best place.

It seems a bit odd to put it in "timefmt" which means time
format, indeed. Perhaps we should rather add them to gnumach's
mach/time_value.h?

Agree. Moved to mach/time_value.h.

Post by Samuel Thibault

Post by Flávio Cruz
--- a/sysdeps/mach/hurd/futimens.c
+++ b/sysdeps/mach/hurd/futimens.c
{
- atime.seconds = tsp[0].tv_sec;
- atime.microseconds = tsp[0].tv_nsec / 1000;
- mtime.seconds = tsp[1].tv_sec;
- mtime.microseconds = tsp[1].tv_nsec / 1000;
+ atime.tv_sec = tsp[0].tv_sec;
+ atime.tv_nsec = tsp[0].tv_nsec;
+ mtime.tv_sec = tsp[1].tv_sec;
+ mtime.tv_nsec = tsp[1].tv_nsec;

Mmm, better just atime=tsp[0]; mtime=tsp[1];?

Post by Flávio Cruz
--- a/sysdeps/mach/hurd/futimes.c
+++ b/sysdeps/mach/hurd/futimes.c
@@ -27,24 +27,44 @@
+ if(tvp == NULL)

Take care of formatting.

Post by Flávio Cruz
--- a/sysdeps/mach/hurd/lutimes.c
+++ b/sysdeps/mach/hurd/lutimes.c
+ if(tvp == NULL)

ditto, and probably other places as well.

Done.

Post by Samuel Thibault

Post by Flávio Cruz
+/* Implement file_utimens as described in <hurd/fs.defs>. */
+kern_return_t
+diskfs_S_file_utimens (struct protid *cred,
+ struct timespec atime,
+ struct timespec mtime)
+{
CHANGE_NODE_FIELD (cred,
- ({
- if (!(err = fshelp_isowner (&np->dn_stat,

cred->user)))

Post by Flávio Cruz
+ ({
+ if (!(err = fshelp_isowner (&np->dn_stat, cred->user)))

Please avoid reformatting indentation even if the existing is odd, it
both makes reading patches clumsier, and makes git annotate to find
changes harder to use.

Post by Flávio Cruz
+ if (atime.tv_nsec == UTIME_NOW)
+ np->dn_set_atime = 1;
+ else if (atime.tv_nsec == UTIME_OMIT)
+ np->dn_set_atime = 0;

I don't think you want to set dn_set_atime to 0 in these places. If there was a
previous utime call which used UTIME_NOW for instance, we don't want to
interfere with that. I think in the UTIME_OMIT case we should just do
nothing.

Gotcha.

Post by Samuel Thibault

Post by Flávio Cruz
diff --git a/libnetfs/file-utimes.c b/libnetfs/file-utimes.c
index 1915609..2bf22f7 100644
--- a/libnetfs/file-utimes.c
+++ b/libnetfs/file-utimes.c
+ if (atimein.tv_nsec == UTIME_NOW || mtimein.tv_nsec == UTIME_NOW)

{

Post by Flávio Cruz
+ maptime_read (netfs_mtime, &t);
+
+ if (atimein.tv_nsec == UTIME_NOW)
+ TIMEVAL_TO_TIMESPEC (&t, &atimein);
+ if (mtimein.tv_nsec == UTIME_NOW)
+ TIMEVAL_TO_TIMESPEC (&t, &mtimein);

}
which will be a bit more efficient, and avoid dumb compilers to emit
warnings about t being undefined.

Done.

Post by Samuel Thibault

Post by Flávio Cruz
--- a/libtreefs/treefs-s-hooks.h
+++ b/libtreefs/treefs-s-hooks.h
@@ -53,7 +53,7 @@ DHH(s_file_chmod, error_t, mode_t)
DHH(s_file_chflags, error_t, int)
#define treefs_s_file_chflags(h, args...)

\

Post by Flávio Cruz
_TREEFS_CHH(h, S_FILE_CHFLAGS, s_file_chflags , ##args)
-DHH(s_file_utimes, error_t, time_value_t, time_value_t)
+DHH(s_file_utimens, error_t, struct timespec, struct timespec)
#define treefs_s_file_utimes(h, args...)

\

Post by Flávio Cruz
_TREEFS_CHH(h, S_FILE_UTIMES, s_file_utimes , ##args)

This looks odd: don't we need both utimes and utimens?

I think we only need file_utimens. Translators that use libtreefs only need
to implement file_utimens. Furthermore, I didn't find any libtreefs based
translator, so we should have no backwards compatibility issue.

Post by Samuel Thibault

Post by Flávio Cruz
diff --git a/libtrivfs/times.c b/libtrivfs/times.c
index 5f08cb1..4c5a0e4 100644
--- a/libtrivfs/times.c
+++ b/libtrivfs/times.c
error_t
trivfs_set_atime (struct trivfs_control *cntl)
{
- io_stat (cntl->underlying, &st);
+ mtime.tv_sec = 0;
+ mtime.tv_nsec = UTIME_OMIT;

that's a nice side effect :)

Post by Flávio Cruz
--- a/nfs/nfs.c
+++ b/nfs/nfs.c
+ else
+ *(p++) = DONT_CHANGE; /* no atime */

also nice :)
Samuel

I've attached the three updated patches.

Flavio

[Samuel Thibault]

Post by Flávio Cruz
CHANGE_NODE_FIELD (cred,
({
if (!(err = fshelp_isowner (&np->dn_stat, cred->user)))
{
- if (atime.microseconds == -1)
+ if (atime.tv_nsec == UTIME_NOW)
np->dn_set_atime = 1;
+ else if (atime.tv_nsec == UTIME_OMIT)
+ ; /* do nothing */

Please take care of the existing indentation, here the code is using
tabs, so please use tabs. Otherwise it looks very odd when using a
different tabset (here you apparently assumed it to be 2, but other
people may be using something else).

Samuel

[Samuel Thibault]

Sorry I didn't think about it at first, but inside the fallback on

+ if (err == MIG_BAD_ID || err == EOPNOTSUPP)
+ {
+ time_value_t atim, mtim;
+
+ if (tsp == NULL)
+ /* Setting the number of microseconds to `-1' tells the
+ underlying filesystems to use the current time. */
+ atim.microseconds = mtim.microseconds = -1;
+ else
+ {
+ TIMESPEC_TO_TIME_VALUE (&atim, &(tsp[0]));
+ TIMESPEC_TO_TIME_VALUE (&mtim, &(tsp[1]));
+ }
+
+ err = HURD_DPORT_USE (fd, __file_utimes (port, atim, mtim));

We should additionally check for tv_nsec being UTIME_OMIT, and in that
case return EOPNOTSUPP. Otherwise we'd be telling old translators a
very odd date.

Samuel

[Flávio Cruz]

Hi Samuel

Post by Samuel Thibault
Sorry I didn't think about it at first, but inside the fallback on

+ if (err == MIG_BAD_ID || err == EOPNOTSUPP)
+ {
+ time_value_t atim, mtim;
+
+ if (tsp == NULL)
+ /* Setting the number of microseconds to `-1' tells the
+ underlying filesystems to use the current time. */
+ atim.microseconds = mtim.microseconds = -1;
+ else
+ {
+ TIMESPEC_TO_TIME_VALUE (&atim, &(tsp[0]));
+ TIMESPEC_TO_TIME_VALUE (&mtim, &(tsp[1]));
+ }
+
+ err = HURD_DPORT_USE (fd, __file_utimes (port, atim, mtim));

We should additionally check for tv_nsec being UTIME_OMIT, and in that
case return EOPNOTSUPP. Otherwise we'd be telling old translators a very
odd date.

Gotcha. I'm also checking for UTIME_NOW and then setting the time_value_t
microseconds field to -1 so that old translators use the current time.

Flávio

Post by Samuel Thibault
Samuel

[Flávio Cruz]

Hi Samuel

Have you see the new patches? Let me know if anything looks wrong.

Cheers
Flavio

Post by Flávio Cruz
Hi Samuel

Post by Samuel Thibault
Sorry I didn't think about it at first, but inside the fallback on

+ if (err == MIG_BAD_ID || err == EOPNOTSUPP)
+ {
+ time_value_t atim, mtim;
+
+ if (tsp == NULL)
+ /* Setting the number of microseconds to `-1' tells the
+ underlying filesystems to use the current time. */
+ atim.microseconds = mtim.microseconds = -1;
+ else
+ {
+ TIMESPEC_TO_TIME_VALUE (&atim, &(tsp[0]));
+ TIMESPEC_TO_TIME_VALUE (&mtim, &(tsp[1]));
+ }
+
+ err = HURD_DPORT_USE (fd, __file_utimes (port, atim, mtim));

We should additionally check for tv_nsec being UTIME_OMIT, and in that
case return EOPNOTSUPP. Otherwise we'd be telling old translators a very
odd date.

Gotcha. I'm also checking for UTIME_NOW and then setting the time_value_t
microseconds field to -1 so that old translators use the current time.
Flávio

Post by Samuel Thibault
Samuel

[Samuel Thibault]

Post by Flávio Cruz
Have you see the new patches?

Yes, it's in my mbox, along so many other mails :/

Samuel

[Samuel Thibault]

Hello,

I (at last) took the time to commit the Hurd part. I'll probably commit
the glibc part soon.

Samuel