If fd_readdir returns a zero inode, call fstatat to get the inode value.
#345
Conversation
| off_t d_ino = entry.d_ino; | ||
| if (d_ino == 0) { | ||
| if (fstatat(dirp->fd, dirent->d_name, &statbuf, AT_SYMLINK_NOFOLLOW) != 0) { | ||
| return NULL; |
There was a problem hiding this comment.
ENOENT etc can happen because of concurrent activities.
aborting on a fstat error here seems like an overreaction to me.
There was a problem hiding this comment.
Good idea about ENOENT. I've made it continue on that.
|
|
||
| // `fd_readdir` implementations may set the inode field to zero if the | ||
| // the inode number is unknown. In that case, do an `fstatat` to get the | ||
| // inode number. |
There was a problem hiding this comment.
is it (ie. d_ino can be 0) going to be a part of the official spec of fd_readdir?
otherwise i'm not sure if it's a good idea to have a workaround here.
There was a problem hiding this comment.
Yes, the idea here would be that we'd document that d_ino can be zero at the WASI level. I've now filed WebAssembly/wasi-filesystem#71 to track this.
|
I think that's an interesting scenario worth covering in https://github.com/WebAssembly/wasi-testsuite, would you mind adding a test for that? |
|
Unfortunately, I don't know of a way to write a test for this with our current infrastructure. It requires an |
d189df4 to
cc65fc0
Compare
… value. On some systems, `fd_readdir` may not implement the `d_ino` field and may set it to zero. When this happens, have wasi-libc call `fstatat` to get the inode number. See the discussion in WebAssembly/wasi-filesystem#65 for details.
cc65fc0 to
efae388
Compare
|
In terms of standardizing this behavior, worst case if we don't standardize it is that wasi-libc makes redundant A potential concern is atomicity. With a separate stat-at call, there is a potential window between when one set of directory entries is read and when their inodes are read. My sense is that this isn't a problem, because directory traversal isn't atomic in general. If there are other processes creating and removing and renaming files in a directory at the same time as a That said, thinking about this led me to think about the case where a file is replaced by another filesystem object with a different type. That's straightforward to handle, so I've now added code to this PR to handle it. |
|
There haven't been any objections to this approach on the spec side, and this doesn't break compatibility, so I'm going to go ahead and land this. |
| if (fstatat(dirp->fd, dirent->d_name, &statbuf, AT_SYMLINK_NOFOLLOW) != 0) { | ||
| if (errno == ENOENT) { | ||
| // The file disappeared before we could read it, so skip it. | ||
| continue; |
There was a problem hiding this comment.
don't you need to advance buffer_processed?
There was a problem hiding this comment.
Good catch! I've now filed #352 to fix that and one other bug.
wasi_snapshot_preview1 recently requires fd_readdir to return actual inode values. On zero, wasi-libc will call fdstat to retrieve them. This introduces our own `platform.Dirent` type and `Readdir` function which a later change will allow fetching of inodes. See WebAssembly/wasi-libc#345 Signed-off-by: Adrian Cole <adrian@tetrate.io>
wasi_snapshot_preview1 recently requires fd_readdir to return actual inode values. On zero, wasi-libc will call fdstat to retrieve them. This introduces our own `platform.Dirent` type and `Readdir` function which a later change will allow fetching of inodes. See WebAssembly/wasi-libc#345 Signed-off-by: Adrian Cole <adrian@tetrate.io>
… value. (WebAssembly#345) * If `fd_readdir` returns a zero inode, call `fstatat` to get the inode value. On some systems, `fd_readdir` may not implement the `d_ino` field and may set it to zero. When this happens, have wasi-libc call `fstatat` to get the inode number. See the discussion in WebAssembly/wasi-filesystem#65 for details. * Update the `d_type` field too, in case it changes.
On some systems,
fd_readdirmay not implement thed_inofield and may set it to zero. When this happens, have wasi-libc callfstatatto get the inode number.See the discussion in
WebAssembly/wasi-filesystem#65 for details.