Skip to content

Commit f552fab

Browse files
alexcrichtonalexcrichton
authored
wasip2: Fix utimensat implementation (#658)
Don't use a WASIp1-centric helper, instead make it WASIp2-specific. Then use the same helper that futimens is using to ensure that both have the same behavior.
1 parent c11f469 commit f552fab

File tree

4 files changed

+66
-111
lines changed

4 files changed

+66
-111
lines changed

‎libc-bottom-half/cloudlibc/src/libc/sys/stat/futimens.c‎

Lines changed: 6 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -15,29 +15,8 @@
1515

1616
#include "stat_impl.h"
1717

18-
#ifdef __wasilibc_use_wasip2
19-
static void set_atim_tag_from_flags(__wasi_fstflags_t flags, filesystem_new_timestamp_t* timestamp) {
20-
if (flags & __WASI_FSTFLAGS_ATIM) {
21-
timestamp->tag = FILESYSTEM_NEW_TIMESTAMP_TIMESTAMP;
22-
} else if (flags & __WASI_FSTFLAGS_ATIM_NOW) {
23-
timestamp->tag = FILESYSTEM_NEW_TIMESTAMP_NOW;
24-
} else {
25-
timestamp->tag = FILESYSTEM_NEW_TIMESTAMP_NO_CHANGE;
26-
}
27-
}
28-
29-
static void set_mtim_tag_from_flags(__wasi_fstflags_t flags, filesystem_new_timestamp_t* timestamp) {
30-
if (flags & __WASI_FSTFLAGS_MTIM) {
31-
timestamp->tag = FILESYSTEM_NEW_TIMESTAMP_TIMESTAMP;
32-
} else if (flags & __WASI_FSTFLAGS_MTIM_NOW) {
33-
timestamp->tag = FILESYSTEM_NEW_TIMESTAMP_NOW;
34-
} else {
35-
timestamp->tag = FILESYSTEM_NEW_TIMESTAMP_NO_CHANGE;
36-
}
37-
}
38-
3918
int futimens(int fd, const struct timespec *times) {
40-
// Translate the file descriptor to an internal handle
19+
#ifdef __wasilibc_use_wasip2
4120
// Translate the file descriptor to an internal handle
4221
filesystem_borrow_descriptor_t file_handle;
4322
if (!fd_to_file_handle_allow_open(fd, &file_handle)) {
@@ -46,22 +25,14 @@ int futimens(int fd, const struct timespec *times) {
4625
}
4726

4827
// Convert timestamps and extract NOW/OMIT flags.
49-
filesystem_datetime_t st_atim;
50-
filesystem_datetime_t st_mtim;
28+
filesystem_new_timestamp_t new_timestamp_atim;
29+
filesystem_new_timestamp_t new_timestamp_mtim;
5130
__wasi_fstflags_t flags;
52-
if (!utimens_get_timestamps(times, &st_atim, &st_mtim, &flags)) {
31+
if (!utimens_get_timestamps(times, &new_timestamp_atim, &new_timestamp_mtim)) {
5332
errno = EINVAL;
5433
return -1;
5534
}
5635

57-
// Set up filesystem_new_timestamps
58-
filesystem_new_timestamp_t new_timestamp_atim;
59-
set_atim_tag_from_flags(flags, &new_timestamp_atim);
60-
new_timestamp_atim.val.timestamp = st_atim;
61-
filesystem_new_timestamp_t new_timestamp_mtim;
62-
set_mtim_tag_from_flags(flags, &new_timestamp_mtim);
63-
new_timestamp_mtim.val.timestamp = st_mtim;
64-
6536
// Perform system call.
6637
filesystem_error_code_t error;
6738
if (!filesystem_method_descriptor_set_times(file_handle,
@@ -72,10 +43,7 @@ int futimens(int fd, const struct timespec *times) {
7243
return -1;
7344
}
7445

75-
return 0;
76-
}
7746
#else
78-
int futimens(int fd, const struct timespec *times) {
7947
// Convert timestamps and extract NOW/OMIT flags.
8048
__wasi_timestamp_t st_atim;
8149
__wasi_timestamp_t st_mtim;
@@ -91,6 +59,7 @@ int futimens(int fd, const struct timespec *times) {
9159
errno = error;
9260
return -1;
9361
}
62+
#endif
63+
9464
return 0;
9565
}
96-
#endif

‎libc-bottom-half/cloudlibc/src/libc/sys/stat/stat_impl.h‎

Lines changed: 26 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -127,53 +127,36 @@ static inline void to_public_stat(const __wasi_filestat_t *in,
127127
#endif
128128

129129
#ifdef __wasilibc_use_wasip2
130+
static inline bool utimens_get_timestamp(const struct timespec *time,
131+
filesystem_new_timestamp_t *out) {
132+
switch (time->tv_nsec) {
133+
case UTIME_NOW:
134+
out->tag = FILESYSTEM_NEW_TIMESTAMP_NOW;
135+
break;
136+
case UTIME_OMIT:
137+
out->tag = FILESYSTEM_NEW_TIMESTAMP_NO_CHANGE;
138+
break;
139+
default:
140+
out->tag = FILESYSTEM_NEW_TIMESTAMP_TIMESTAMP;
141+
if (!timespec_to_timestamp_exact(time, &out->val.timestamp))
142+
return false;
143+
break;
144+
}
145+
return true;
146+
}
147+
130148
static inline bool utimens_get_timestamps(const struct timespec *times,
131-
filesystem_datetime_t *st_atim,
132-
filesystem_datetime_t *st_mtim,
133-
__wasi_fstflags_t *flags) {
149+
filesystem_new_timestamp_t *st_atim,
150+
filesystem_new_timestamp_t *st_mtim) {
134151
if (times == NULL) {
135152
// Update both timestamps.
136-
*flags = __WASI_FSTFLAGS_ATIM_NOW | __WASI_FSTFLAGS_MTIM_NOW;
137-
st_atim->seconds = 0;
138-
st_atim->nanoseconds = 0;
139-
st_mtim->seconds = 0;
140-
st_mtim->nanoseconds = 0;
153+
st_atim->tag = FILESYSTEM_NEW_TIMESTAMP_NOW;
154+
st_mtim->tag = FILESYSTEM_NEW_TIMESTAMP_NOW;
141155
} else {
142-
// Set individual timestamps.
143-
*flags = 0;
144-
switch (times[0].tv_nsec) {
145-
case UTIME_NOW:
146-
*flags |= __WASI_FSTFLAGS_ATIM_NOW;
147-
st_atim->seconds = 0;
148-
st_atim->nanoseconds = 0;
149-
break;
150-
case UTIME_OMIT:
151-
st_atim->seconds = 0;
152-
st_atim->nanoseconds = 0;
153-
break;
154-
default:
155-
*flags |= __WASI_FSTFLAGS_ATIM;
156-
if (!timespec_to_timestamp_exact(&times[0], st_atim))
157-
return false;
158-
break;
159-
}
160-
161-
switch (times[1].tv_nsec) {
162-
case UTIME_NOW:
163-
*flags |= __WASI_FSTFLAGS_MTIM_NOW;
164-
st_mtim->seconds = 0;
165-
st_mtim->nanoseconds = 0;
166-
break;
167-
case UTIME_OMIT:
168-
st_mtim->seconds = 0;
169-
st_mtim->nanoseconds = 0;
170-
break;
171-
default:
172-
*flags |= __WASI_FSTFLAGS_MTIM;
173-
if (!timespec_to_timestamp_exact(&times[1], st_mtim))
174-
return false;
175-
break;
176-
}
156+
if (!utimens_get_timestamp(&times[0], st_atim))
157+
return false;
158+
if (!utimens_get_timestamp(&times[1], st_mtim))
159+
return false;
177160
}
178161
return true;
179162
}

‎libc-bottom-half/cloudlibc/src/libc/sys/stat/utimensat.c‎

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,9 @@
1717

1818
#include "stat_impl.h"
1919

20-
#ifdef __wasilibc_use_wasip2
2120
int __wasilibc_nocwd_utimensat(int fd, const char *path, const struct timespec times[2],
2221
int flag) {
23-
// Translate the file descriptor to an internal handle
22+
#ifdef __wasilibc_use_wasip2
2423
// Translate the file descriptor to an internal handle
2524
filesystem_borrow_descriptor_t file_handle;
2625
if (!fd_to_file_handle_allow_open(fd, &file_handle)) {
@@ -29,22 +28,13 @@ int __wasilibc_nocwd_utimensat(int fd, const char *path, const struct timespec t
2928
}
3029

3130
// Convert timestamps and extract NOW/OMIT flags.
32-
filesystem_datetime_t st_atim;
33-
filesystem_datetime_t st_mtim;
34-
__wasi_fstflags_t flags;
35-
if (!utimens_get_timestamps(times, &st_atim, &st_mtim, &flags)) {
31+
filesystem_new_timestamp_t new_timestamp_atim;
32+
filesystem_new_timestamp_t new_timestamp_mtim;
33+
if (!utimens_get_timestamps(times, &new_timestamp_atim, &new_timestamp_mtim)) {
3634
errno = EINVAL;
3735
return -1;
3836
}
3937

40-
// Set up filesystem_new_timestamps
41-
filesystem_new_timestamp_t new_timestamp_atim;
42-
new_timestamp_atim.tag = FILESYSTEM_NEW_TIMESTAMP_TIMESTAMP;
43-
new_timestamp_atim.val.timestamp = st_atim;
44-
filesystem_new_timestamp_t new_timestamp_mtim;
45-
new_timestamp_mtim.tag = FILESYSTEM_NEW_TIMESTAMP_TIMESTAMP;
46-
new_timestamp_mtim.val.timestamp = st_mtim;
47-
4838
// Create lookup properties.
4939
__wasi_lookupflags_t lookup_flags = 0;
5040
if ((flag & AT_SYMLINK_NOFOLLOW) == 0)
@@ -68,11 +58,8 @@ int __wasilibc_nocwd_utimensat(int fd, const char *path, const struct timespec t
6858
return -1;
6959
}
7060

71-
return 0;
72-
}
7361
#else
74-
int __wasilibc_nocwd_utimensat(int fd, const char *path, const struct timespec times[2],
75-
int flag) {
62+
7663
// Convert timestamps and extract NOW/OMIT flags.
7764
__wasi_timestamp_t st_atim;
7865
__wasi_timestamp_t st_mtim;
@@ -94,6 +81,6 @@ int __wasilibc_nocwd_utimensat(int fd, const char *path, const struct timespec t
9481
errno = error;
9582
return -1;
9683
}
84+
#endif
9785
return 0;
9886
}
99-
#endif

‎test/src/utime.c‎

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <stdlib.h>
1010
#include <unistd.h>
1111
#include <fcntl.h>
12+
#include <sys/time.h>
1213
#include "test.h"
1314

1415
#define TEST(c, ...) ((c) ? 1 : (t_error(#c" failed: " __VA_ARGS__),0))
@@ -24,27 +25,32 @@ static FILE *make_temp_file() {
2425
return f;
2526
}
2627

27-
int main(void)
28+
static int doit_futimens(int fd, const struct timespec times[2]) {
29+
return futimens(fd, times);
30+
}
31+
32+
static int doit_utimensat(int fd, const struct timespec times[2]) {
33+
return utimensat(AT_FDCWD, "temp_file", times, 0);
34+
}
35+
36+
void run(int (*set_times)(int, const struct timespec[2]))
2837
{
2938
struct stat st;
3039
FILE *f;
3140
int fd;
3241
time_t t;
3342

34-
TEST(futimens(-1, ((struct timespec[2]){{.tv_nsec=UTIME_OMIT},{.tv_nsec=UTIME_OMIT}}))==0 || errno==EBADF,
35-
"%s\n", strerror(errno));
36-
37-
if (!TEST(f = make_temp_file())) return t_status;
43+
if (!TEST(f = make_temp_file())) return;
3844
fd = fileno(f);
3945

40-
TEST(futimens(fd, (struct timespec[2]){0}) == 0, "\n");
46+
TEST(set_times(fd, (struct timespec[2]){0}) == 0, "\n");
4147
TEST(fstat(fd, &st) == 0, "\n");
4248
TESTVAL(st.st_atim.tv_sec,==,0);
4349
TESTVAL(st.st_atim.tv_nsec,==,0);
4450
TESTVAL(st.st_mtim.tv_sec,==,0);
4551
TESTVAL(st.st_mtim.tv_nsec,==,0);
4652

47-
TEST(futimens(fd, ((struct timespec[2]){{.tv_sec=1,.tv_nsec=UTIME_OMIT},{.tv_sec=1,.tv_nsec=UTIME_OMIT}})) == 0, "\n");
53+
TEST(set_times(fd, ((struct timespec[2]){{.tv_sec=1,.tv_nsec=UTIME_OMIT},{.tv_sec=1,.tv_nsec=UTIME_OMIT}})) == 0, "\n");
4854
TEST(fstat(fd, &st) == 0, "\n");
4955
TESTVAL(st.st_atim.tv_sec,==,0);
5056
TESTVAL(st.st_atim.tv_nsec,==,0);
@@ -53,32 +59,42 @@ int main(void)
5359

5460
t = time(0);
5561

56-
TEST(futimens(fd, ((struct timespec[2]){{.tv_nsec=UTIME_NOW},{.tv_nsec=UTIME_OMIT}})) == 0, "\n");
62+
TEST(set_times(fd, ((struct timespec[2]){{.tv_nsec=UTIME_NOW},{.tv_nsec=UTIME_OMIT}})) == 0, "\n");
5763
TEST(fstat(fd, &st) == 0, "\n");
5864
TESTVAL(st.st_atim.tv_sec,>=,t);
5965
TESTVAL(st.st_mtim.tv_sec,==,0);
6066
TESTVAL(st.st_mtim.tv_nsec,==,0);
6167

62-
TEST(futimens(fd, (struct timespec[2]){0}) == 0, "\n");
63-
TEST(futimens(fd, ((struct timespec[2]){{.tv_nsec=UTIME_OMIT},{.tv_nsec=UTIME_NOW}})) == 0, "\n");
68+
TEST(set_times(fd, (struct timespec[2]){0}) == 0, "\n");
69+
TEST(set_times(fd, ((struct timespec[2]){{.tv_nsec=UTIME_OMIT},{.tv_nsec=UTIME_NOW}})) == 0, "\n");
6470
TEST(fstat(fd, &st) == 0, "\n");
6571
TESTVAL(st.st_atim.tv_sec,==,0);
6672
TESTVAL(st.st_mtim.tv_sec,>=,t);
6773

68-
TEST(futimens(fd, ((struct timespec[2]){{.tv_nsec=UTIME_NOW},{.tv_nsec=UTIME_OMIT}})) == 0, "\n");
74+
TEST(set_times(fd, ((struct timespec[2]){{.tv_nsec=UTIME_NOW},{.tv_nsec=UTIME_OMIT}})) == 0, "\n");
6975
TEST(fstat(fd, &st) == 0, "\n");
7076
TESTVAL(st.st_atim.tv_sec,>=,t);
7177
TESTVAL(st.st_mtim.tv_sec,>=,t);
7278

7379
if (TEST((time_t)(1LL<<32) == (1LL<<32), "implementation has Y2038 EOL\n")) {
74-
if (TEST(futimens(fd, ((struct timespec[2]){{.tv_sec=1LL<<32},{.tv_sec=1LL<<32}})) == 0, "%s\n", strerror(errno))) {
80+
if (TEST(set_times(fd, ((struct timespec[2]){{.tv_sec=1LL<<32},{.tv_sec=1LL<<32}})) == 0, "%s\n", strerror(errno))) {
7581
TEST(fstat(fd, &st) == 0, "\n");
7682
TESTVAL(st.st_atim.tv_sec, ==, 1LL<<32);
7783
TESTVAL(st.st_mtim.tv_sec, ==, 1LL<<32);
7884
}
7985
}
8086

8187
fclose(f);
88+
}
89+
90+
int main(void)
91+
{
92+
TEST(futimens(-1, ((struct timespec[2]){{.tv_nsec=UTIME_OMIT},{.tv_nsec=UTIME_OMIT}}))==0 || errno==EBADF,
93+
"%s\n", strerror(errno));
94+
fprintf(stderr, "testing futimens...\n");
95+
run(doit_futimens);
96+
fprintf(stderr, "testing utimensat...\n");
97+
run(doit_utimensat);
8298

8399
return t_status;
84100
}

0 commit comments

Comments
 (0)