Skip to content

Refactor fd.rs return_read_bytes_and_count and return_written_byte_count_or_error #3904

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

Closed
tiif opened this issue Sep 22, 2024 · 4 comments · Fixed by #3923
Closed

Refactor fd.rs return_read_bytes_and_count and return_written_byte_count_or_error #3904

tiif opened this issue Sep 22, 2024 · 4 comments · Fixed by #3923
Assignees

Comments

@tiif
Copy link
Contributor

tiif commented Sep 22, 2024

It'd be clearer if return_read_bytes_and_count and return_written_byte_count_or_error can be combined and separate according to the Ok and Err code path. This is mentioned in #3852 (comment).

(This probably don't need to be an issue, but I am creating this to keep track of what I want to do.)

@tiif
Copy link
Contributor Author

tiif commented Sep 22, 2024

@rustbot claim

@RalfJung
Copy link
Member

I'd phrase this more generally as -- have a nice helper for returning errors in the standard Unix way.

@tiif
Copy link
Contributor Author

tiif commented Sep 22, 2024

I'd phrase this more generally as -- have a nice helper for returning errors in the standard Unix way.

Actually there is try_unwrap_io_result:

    fn try_unwrap_io_result<T: From<i32>>(
        &mut self,
        result: std::io::Result<T>,
    ) -> InterpResult<'tcx, T> {
        match result {
            Ok(ok) => Ok(ok),
            Err(e) => {
                self.eval_context_mut().set_last_error_from_io_error(e)?;
                Ok((-1).into())
            }
        }
    }

So do we want to have another helper function?

@RalfJung
Copy link
Member

RalfJung commented Sep 22, 2024

Yes that one is nice, but I think we want something "lower-level" that takes an io::Error rather than an io::Result. And that does the write to dest as well.

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 a pull request may close this issue.

2 participants