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

Return __WASI_EINVAL from fd_prestat_dir_name #2580

Merged
merged 1 commit into from
Sep 25, 2023

Conversation

zoraaver
Copy link
Contributor

@zoraaver zoraaver commented Sep 21, 2023

Return a WASI error code (rather than a host POSIX one). In addition,
there is no need to return an error in the case that the provided buffer
is too large.

@yamt
Copy link
Collaborator

yamt commented Sep 21, 2023

The error code ENAMETOOLONG is a more accurate description of the error condition if the provided path buffer is too short to fit the prestat directory name. In addition, there is no need to return an error in the case that the provided buffer is too large.

ENAMETOOLONG sounds weird to me for this particular case.

@zoraaver
Copy link
Contributor Author

@yamt I think the error code is fine. From the posix spec page Filename too long is the description.

I think this matches the error condition here pretty much exactly: the filename is too long to fit in the provided buffer.

@yamt
Copy link
Collaborator

yamt commented Sep 22, 2023

@yamt I think the error code is fine. From the posix spec page Filename too long is the description.

I think this matches the error condition here pretty much exactly: the filename is too long to fit in the provided buffer.

ENAMETOOLONG usually means that the given filename is too long wrt PATH_MAX/NAME_MAX.
do you have a precedent where it's used to mean "too long for the user-provided buffer"?

@loganek
Copy link
Collaborator

loganek commented Sep 22, 2023

ENAMETOOLONG sounds weird to me for this particular case.

Agree, I think EINVAL is fine in that case, as it's invalid value provided by the user that causes this error.

@zoraaver
Copy link
Contributor Author

ENAMETOOLONG usually means that the given filename is too long wrt PATH_MAX/NAME_MAX.

Ok gotcha, I hadn't actually come across the error code before so just from the name and description it seemed to fit well.

do you have a precedent where it's used to mean "too long for the user-provided buffer"?

It appears not, I've searched the internet but the error code seems to refer exclusively to that specific case of a path exceeding the OS path max.

I'll still need to change this function to return __WASI_EINVAL rather than __EINVAL. I'll also raise a PR for the tests to change the error code.

@zoraaver zoraaver changed the title Return ENAMETOOLONG from fd_prestat_dir_name Return __WASI_EINVAL from fd_prestat_dir_name Sep 22, 2023
Return a WASI error code (rather than a host POSIX one). In addition,
there is no need to return an error in the case that the provided buffer
is too large.
Copy link
Contributor

@wenyongh wenyongh left a comment

Choose a reason for hiding this comment

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

LGTM

@wenyongh wenyongh merged commit f474f3d into bytecodealliance:main Sep 25, 2023
368 checks passed
@zoraaver zoraaver deleted the correct-error-code branch September 25, 2023 11:02
victoryang00 pushed a commit to victoryang00/wamr-aot-gc-checkpoint-restore that referenced this pull request May 27, 2024
Return a WASI error code (rather than a host POSIX one). In addition,
there is no need to return an error in the case that the provided buffer
is too large.
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.

4 participants