zig icon indicating copy to clipboard operation
zig copied to clipboard

std.posix: handle INVAL in openZ, openatZ and openatWasi

Open tealsnow opened this issue 1 year ago • 4 comments

Contributes to #15607

~~Although the case is not handled in openatWasi (as I could not get a working wasi environment to test the change) I have added a FIXME addressing it and linking to the issue.~~

Edit: Handled case in openatWasi too

tealsnow avatar May 01 '24 19:05 tealsnow

Using the createfile.zig example from https://github.com/ziglang/zig/issues/15607 on a FAT filesystem, compiled with:

zig build-exe createfile.zig -target wasm32-wasi

Ran with:

wasmtime --dir . createfile.wasm

(note: openatWasi is used for Dir.createFile)

  • When the host is Windows, NOENT is returned from wasi.path_open (on both NTFS and FAT filesystems)
  • When the host is Linux, INVAL is returned from wasi.path_open

So, .INVAL => return error.BadPathName, seems like the way to go for now in openatWasi


Side note: INVAL can be returned for more reasons than just characters that are unsupported by the underlying filesystem. We might eventually want to take that into account, but I think mapping it to BadPathName for now is fine.

squeek502 avatar May 01 '24 23:05 squeek502

Side note: INVAL can be returned for more reasons than just characters that are unsupported by the underlying filesystem. We might eventually want to take that into account, but I think mapping it to BadPathName for now is fine.

I'm interested in investigating these other cases and handling them properly. I found some documentation on such cases here. Are you aware of any other cases?

tealsnow avatar May 02 '24 08:05 tealsnow

Those are the cases I was referring to. However, each operating system may also behave differently; that link is just for Linux.

For example, it looks like FreeBSD doesn't document what happens in this case (the only documented EINVAL return is about invalid flag combinations). Would likely need to test mounting a FAT32 drive and seeing what happens when you try to create a file with an illegal character.

So, the idea I had in mind would be something like:

  • Check the documented EINVAL cases for as many operating systems as possible
  • Check the actual behavior of attempting to create a file with an illegal character on a mounted FAT32 drive on as many operating systems as possible

Then, we'd try figure out if there are any commonalities, and potentially whether or not it would make sense to translate EINVAL to a broader error than BadPathName.

It also might make sense to handle EINVAL with a switch, something like:

.INVAL => switch (native_os) {
    .linux => return error.BadPathName,
    .freebsd => unreachable, // just a hypothetical, if it turns out that FreeBSD doesn't use INVAL for the illegal character case
    // etc
},

but, again, I feel like we would need more information about how different OSes handle the situation to determine what the switch cases should actually do. In the meantime, I think the changes in this PR are a step in the right direction.


These are just my initial thoughts on this, though. Addressing https://github.com/ziglang/zig/issues/15607 has been on my todo list for a while but I haven't actually put any work into it yet.

squeek502 avatar May 02 '24 09:05 squeek502

Thanks!

andrewrk avatar Jul 21 '24 09:07 andrewrk

Just to clarify what happened here: this PR was rebased and auto-merge was turned on, but master was failing CI at the time. I just rebased again and CI passed so it was auto-merged.

squeek502 avatar Jul 29 '24 05:07 squeek502