Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add expected error code for fd_prestat_dir_name #92

Merged
merged 1 commit into from
Oct 3, 2023

Conversation

zoraaver
Copy link
Contributor

@zoraaver zoraaver commented Sep 22, 2023

In the case that the user provides a buffer which is too small to fit the directory name, it makes more sense to return EINVAL rather than ENAMETOOLONG. ENAMETOOLONG has the speficic connotation that a path was longer than the maximum size permitted by the OS - that doesn't really match this error condition. We're keeping both error codes for now since we don't specify in the docs what error code should be returned.

Copy link
Collaborator

@loganek loganek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yamt
Copy link
Contributor

yamt commented Sep 27, 2023

is it strictly mentioned in the spec?
otherwise, i guess it's safer to allow both error codes.
wasmtime returns ENAMETOOLONG, doesn't it?

@zoraaver
Copy link
Contributor Author

It's not mentioned in the spec, https://docs.rs/wasi/latest/wasi/fn.fd_prestat_dir_name.html. But I'd say it's better to standardise on a single error code. In this case it shouldn't be much work to change it either way. As far as I understand, the purpose of the tests isn't so that they pass immediately on all runtimes, it's to highlight and standardise any differences in behaviour.

wasmtime returns ENAMETOOLONG, doesn't it?

It probably does since this test is originally from the wasmtime set of tests.

I've raised WebAssembly/WASI#555 to discuss standardising error codes from filesystem functions in general.

@yamt
Copy link
Contributor

yamt commented Sep 27, 2023

It's not mentioned in the spec, https://docs.rs/wasi/latest/wasi/fn.fd_prestat_dir_name.html. But I'd say it's better to standardise on a single error code. In this case it shouldn't be much work to change it either way. As far as I understand, the purpose of the tests isn't so that they pass immediately on all runtimes, it's to highlight and standardise any differences in behaviour.

my point is such a standardization should happen in spec, not tests.

wasmtime returns ENAMETOOLONG, doesn't it?

It probably does since this test is originally from the wasmtime set of tests.

I've raised WebAssembly/WASI#555 to discuss standardising error codes from filesystem functions in general.

i see.

In the case that the user provides a buffer which is too small to fit
the directory name, it makes more sense to return EINVAL rather than
ENAMETOOLONG. ENAMETOOLONG has the speficic connotation that a path was
longer than the maximum size permitted by the OS - that doesn't really
match this error condition. We're keeping both error codes for now since
we don't specify in the docs what error code should be returned.
@zoraaver zoraaver changed the title Change expected error code from fd_prestat_dir_name Add expected error code for fd_prestat_dir_name Oct 3, 2023
@zoraaver
Copy link
Contributor Author

zoraaver commented Oct 3, 2023

my point is such a standardization should happen in spec, not tests.

True, I've kept both error codes until we decide how to proceed with standardising error codes in general.

@loganek loganek merged commit 2ea824c into WebAssembly:main Oct 3, 2023
11 checks passed
@zoraaver zoraaver deleted the correct-error-code branch October 3, 2023 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants