Skip to content

implemented BufWriter #10451

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

Merged
merged 1 commit into from
Nov 13, 2013
Merged

implemented BufWriter #10451

merged 1 commit into from
Nov 13, 2013

Conversation

zkamsler
Copy link
Contributor

I implemented BufWriter. I realize the use of conditions are on their way out for IO, but it does raise a condition if a write will not fit in the buffer for now.

I also replaced the seek code for MemWriter. It was adding the offset as a uint, which is unsound for negative offsets. It only happened to work because unsigned addition performs the same operation with two's complement, and sizeof(uint) <= sizeof(i64) so there was no (lack of) sign extension. I replaced this with computing an offset as an i64 and clamping to zero. I don't expect anyone will have use BufWriter with a byte buffer greater than 2^63 bytes any time soon.

@alexcrichton

Closes #10433

@alexcrichton
Copy link
Member

Interesting, and cool! A few things:

  • Can you tag the Seek implementations with FIXME(#10432) to signify that they might change?
  • Can you document what happens when you perform a write that goes out of bounds?
  • Can you add tests which do writes out of bounds?
  • Can you add tests which seek?
  • Could you expand the commit message to be a bit more descriptive?

Thanks again!

@zkamsler
Copy link
Contributor Author

@alexcrichton
Sure. Should the out of bounds documentation go with the struct definition or the write method in the trait implementation? I am not fully up on the conventions for rust documentation.

@alexcrichton
Copy link
Member

Let's put it on either the struct definition or the constructor for now.

Filled in the implementations of Writer and Seek for BufWriter. It
raises the io_error condition if a write cannot fit in the buffer.

The Seek implementation for MemWriter, which was incorrectly using
unsigned arithmatic to add signed offsets, has also been replaced.
@zkamsler
Copy link
Contributor Author

@alexcrichton
I have updated the commit as per your comments.

bors added a commit that referenced this pull request Nov 13, 2013
I implemented BufWriter. I realize the use of conditions are on their way out for IO, but it does raise a condition if a write will not fit in the buffer for now.

I also replaced the seek code for MemWriter. It was adding the offset as a uint, which is unsound for negative offsets. It only happened to work because unsigned addition performs the same operation with two's complement, and sizeof(uint) <= sizeof(i64) so there was no (lack of) sign extension. I replaced this with computing an offset as an i64 and clamping to zero. I don't expect anyone will have use BufWriter with a byte buffer greater than 2^63 bytes any time soon.

@alexcrichton

Closes #10433
@bors bors closed this Nov 13, 2013
@bors bors merged commit c2c7a24 into rust-lang:master Nov 13, 2013
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.

BufWriter needs an implementation or removal
3 participants