Skip to content

Commit

Permalink
Change readdir from returning bytes to returning a dir-entrys. (#72)
Browse files Browse the repository at this point in the history
`readdir` returning bytes has been a source of complexity for both libc
and engine implementors, with subtle issues about alignment and partial
records.

Rename `dirent` to `dir-entry` for readability, and revamp it:
 - Make the name field a proper `string` rather than being represented
   by trailing bytes in the buffer.
 - Make the inode field `optional`, so that we can make it optional
   as discussed in #65 without special-casing zero, which is reportedly
   a valid inode number on [some filesystems].

And remove the `rewind` parameter, which isn't needed when we return
a stream, as users wanting to start at the beginning can just request
a new stream.

[some filesystems]: https://sourceware.org/pipermail/libc-alpha/2022-September/142059.html
  • Loading branch information
sunfishcode authored Dec 7, 2022
1 parent b8a6e5a commit 1d3e556
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 38 deletions.
35 changes: 16 additions & 19 deletions wasi-filesystem.abi.md
Original file line number Diff line number Diff line change
Expand Up @@ -268,25 +268,30 @@ Size: 16, Alignment: 8

Set the timestamp to the given value.

## <a href="#dirent" name="dirent"></a> `dirent`: record
## <a href="#dir_entry" name="dir_entry"></a> `dir-entry`: record

A directory entry.

Size: 16, Alignment: 8
Size: 32, Alignment: 8

### Record Fields

- <a href="dirent.ino" name="dirent.ino"></a> [`ino`](#dirent.ino): [`inode`](#inode)
- <a href="dir_entry.ino" name="dir_entry.ino"></a> [`ino`](#dir_entry.ino): option<[`inode`](#inode)>

The serial number of the file referred to by this directory entry.
The serial number of the object referred to by this directory entry.
May be none if the inode value is not known.

When this is none, libc implementations might do an extra `stat-at`
call to retrieve the inode number to fill their `d_ino` fields, so
implementations which can set this to a non-none value should do so.

- <a href="dirent.namelen" name="dirent.namelen"></a> [`namelen`](#dirent.namelen): [`size`](#size)
- <a href="dir_entry.type" name="dir_entry.type"></a> [`type`](#dir_entry.type): [`descriptor-type`](#descriptor_type)

The length of the name of the directory entry.
The type of the file referred to by this directory entry.

- <a href="dirent.type" name="dirent.type"></a> [`type`](#dirent.type): [`descriptor-type`](#descriptor_type)
- <a href="dir_entry.name" name="dir_entry.name"></a> [`name`](#dir_entry.name): `string`

The type of the file referred to by this directory entry.
The name of the object.

## <a href="#errno" name="errno"></a> `errno`: enum

Expand Down Expand Up @@ -787,22 +792,14 @@ Size: 16, Alignment: 8

Read directory entries from a directory.

When successful, the contents of the output buffer consist of a sequence of
directory entries. Each directory entry consists of a `dirent` object,
followed by `dirent::d_namlen` bytes holding the name of the directory
entry.

This function fills the output buffer as much as possible, potentially
truncating the last directory entry. This allows the caller to grow its
read buffer size in case it's too small to fit a single large directory
entry, or skip the oversized directory entry.
This always returns a new stream which starts at the beginning of the
directory.
##### Params

- <a href="#descriptor_readdir.self" name="descriptor_readdir.self"></a> `self`: handle<descriptor>
- <a href="#descriptor_readdir.rewind" name="descriptor_readdir.rewind"></a> `rewind`: `bool`
##### Results

- stream<`u8`, [`errno`](#errno)>
- stream<[`dir-entry`](#dir_entry), [`errno`](#errno)>

----

Expand Down
35 changes: 16 additions & 19 deletions wasi-filesystem.wit.md
Original file line number Diff line number Diff line change
Expand Up @@ -192,16 +192,23 @@ variant new-timestamp {
}
```

## `dirent`
## `dir-entry`
```wit
/// A directory entry.
record dirent {
/// The serial number of the file referred to by this directory entry.
ino: inode,
/// The length of the name of the directory entry.
namelen: size,
record dir-entry {
/// The serial number of the object referred to by this directory entry.
/// May be none if the inode value is not known.
///
/// When this is none, libc implementations might do an extra `stat-at`
/// call to retrieve the inode number to fill their `d_ino` fields, so
/// implementations which can set this to a non-none value should do so.
ino: option<inode>,
/// The type of the file referred to by this directory entry.
%type: descriptor-type,
/// The name of the object.
name: string,
}
```

Expand Down Expand Up @@ -506,19 +513,9 @@ pwrite: func(
```wit
/// Read directory entries from a directory.
///
/// When successful, the contents of the output buffer consist of a sequence of
/// directory entries. Each directory entry consists of a `dirent` object,
/// followed by `dirent::d_namlen` bytes holding the name of the directory
/// entry.
///
/// This function fills the output buffer as much as possible, potentially
/// truncating the last directory entry. This allows the caller to grow its
/// read buffer size in case it's too small to fit a single large directory
/// entry, or skip the oversized directory entry.
readdir: func(
/// If true, rewind the current position to the beginning before reading.
rewind: bool
) -> stream<u8, errno>
/// This always returns a new stream which starts at the beginning of the
/// directory.
readdir: func() -> stream<dir-entry, errno>
```

## `seek`
Expand Down

0 comments on commit 1d3e556

Please sign in to comment.