futimens fails (causes FileUtilTest.TouchFile failure on Fuchsia) |
||
Issue description
#include <fcntl.h>
#include <sys/time.h>
#include <magenta/syscalls.h>
int main(int argc, char** argv) {
int fd = open("/tmp/file.txt", O_RDWR|O_CREAT, 0644);
fprintf(stderr, "open, fd=%d\n", fd);
struct timespec ts[2] = {};
ts[0].tv_nsec = UTIME_OMIT;
int64_t now = mx_time_get(MX_CLOCK_UTC);
ts[1].tv_sec = now / MX_SEC(1);
ts[1].tv_nsec = now % MX_SEC(1);
int ret = futimens(fd, ts);
fprintf(stderr, "futimens() ret=%d\n", ret);
close(fd);
return 0;
}
on Fuchsia, I get:
[00000.614] 02379.02598> open, fd=3
[00000.614] 02379.02598> futimens() ret=-1
ls -l /tmp is:
[00000.618] 01645.02208> d 2 0 .
[00000.618] 01645.02208> d 1 0 ..
[00000.618] 01645.02208> - 1 0 file.txt
I feel like I must be missing something obvious, but I don't understand what.
,
Jun 20 2017
Or maybe easier, futimes() example that works on Linux for comparison.
#include <stdio.h>
#include <fcntl.h>
#include <sys/time.h>
#include <unistd.h>
int main(int argc, char** argv) {
int fd = open("/tmp/file.txt", O_RDWR|O_CREAT, 0644);
fprintf(stderr, "open, fd=%d\n", fd);
timeval times[2];
times[0].tv_sec = 0;
times[0].tv_usec = 0;
times[1].tv_sec = 0;
times[1].tv_usec = 0;
int ret = futimes(fd, times);
fprintf(stderr, "futimes() ret=%d\n", ret);
close(fd);
return 0;
}
,
Jun 20 2017
I can investigate; it seems like there might be a couple problems here. 1) Obviously, one of the futimens needs to go. I think orr@ was unaware of the one in musl when he added one to unistd. 2) The fact that the futimens call in musl isn't working is a sign that "utimensat" may have an issue. I'll try to fix + attach a test to prevent regressions.
,
Jun 21 2017
FWIW, "system/utest/fs/test-attr.c" tests almost the same behavior as your first program. I suspect it's calling musl's implementation. musl's "futimens" calls "utimensat" with NULL as an argument for "pathname". To quote: http://man7.org/linux/man-pages/man2/utimensat.2.html C library/ kernel ABI differences On Linux, futimens() is a library function implemented on top of the utimensat() system call. To support this, the Linux utimensat() system call implements a nonstandard feature: if pathname is NULL, then the call modifies the timestamps of the file referred to by the file descriptor dirfd (which may refer to any type of file). Using this feature, the call futimens(fd, times) is implemented as: utimensat(fd, NULL, times, 0); Note, however, that the glibc wrapper for utimensat() disallows passing NULL as the value for file: the wrapper function returns the error EINVAL in this case. To avoid exposing this odd behavior out of "utimensat" (which, for us, is implemented in unistd.c, rather than a syscall), I plan on eliminating the musl implementation of futimens. This means that to use "futimens", your code must link against libmxio. George, is that okay with you? https://fuchsia-review.googlesource.com/c/35564/
,
Jun 21 2017
sgtm
,
Jun 21 2017
ref: https://fuchsia-review.googlesource.com/c/35564/ Thanks! Just pulled it in, and the touch test works* in Chromium now. * modification time works, access time looks like it's not implemented.
,
Jun 21 2017
(Does access time make sense? Should I file an upstream bug for implementing that?)
,
Jun 21 2017
We've been mulling around the possibility of access time, but I think we are leaning towards not implementing it -- it's a pretty expensive operation, which has limited benefits. Modification / Creation time should still be supported.
,
Jun 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1e15c66857e9d4ebc0bf3a5e13ca1c7ba29eea97 commit 1e15c66857e9d4ebc0bf3a5e13ca1c7ba29eea97 Author: Scott Graham <scottmg@chromium.org> Date: Wed Jun 21 21:07:29 2017 Roll Fuchsia SDK to 457adecd1600ab82efa6d142cc506560b7d8f7c8 Follows https://bugs.chromium.org/p/chromium/issues/detail?id=707030#c4. Notable changes: - Includes https://fuchsia-review.googlesource.com/c/35564/ which makes futime[n]s() work, except that access time is not supported on Fuchsia (see linked bug). So FileUtilTest.TouchFile mostly works now. - Includes https://fuchsia.googlesource.com/magenta/+/2db38e2b99162c84ae43d39715031d59fcee7ac5 which removes '..' from readdir() (see other linked bug). For this case, update FileEnumerator to include a fake .. to preserve the interface expected for INCLUDE_DOT_DOT. This is necessary to keep FileUtilTest.FileEnumeratorTest working. (yay! caught a semi- regression) Bug: 706592, 735233 , 735540 Change-Id: Iafbd0f80ef386c2def9edd5be3e309b034cdf215 Reviewed-on: https://chromium-review.googlesource.com/542945 Reviewed-by: Nico Weber <thakis@chromium.org> Commit-Queue: Scott Graham <scottmg@chromium.org> Cr-Commit-Position: refs/heads/master@{#481293} [modify] https://crrev.com/1e15c66857e9d4ebc0bf3a5e13ca1c7ba29eea97/DEPS [modify] https://crrev.com/1e15c66857e9d4ebc0bf3a5e13ca1c7ba29eea97/base/files/file_enumerator_posix.cc [modify] https://crrev.com/1e15c66857e9d4ebc0bf3a5e13ca1c7ba29eea97/base/files/file_util_unittest.cc [modify] https://crrev.com/1e15c66857e9d4ebc0bf3a5e13ca1c7ba29eea97/testing/buildbot/filters/fuchsia.base_unittests.filter |
||
►
Sign in to add a comment |
||
Comment 1 by scottmg@chromium.org
, Jun 20 2017