Skip to content

Conversation

arjantop
Copy link
Contributor

First condition is not needed and just prevents 0 length writes

Fixes #15583

@alexcrichton
Copy link
Member

Could you add a test for this as well?

@arjantop
Copy link
Contributor Author

Yeah just remembered, working on that

@bluss
Copy link
Contributor

bluss commented Jul 10, 2014

You'd write buf.len() > max_size - self.pos to be safe against overflow. I'm not sure if the surrounding code is written with that in mind, or if it matters, though.

(re-post comment from changed diff to here)

@arjantop
Copy link
Contributor Author

Added test and changed condition to @blake2-ppc suggestion

@arjantop
Copy link
Contributor Author

Looks like there ia a problem with a @blake2-ppc suggestion because seeking beyond end of buffer is allowed and all the lengths are unsigned.

@bluss
Copy link
Contributor

bluss commented Jul 11, 2014

True, I'm sorry for that; the conditional needs an invariant that Seek doesn't keep in place. The fact remains that the original condition is an "overflow trigger", a pattern that usually has an overflow problem.

In the past I fixed a lot of overflow problems with vectors -- Rust code would write overflowing calculations and head directly into unsafe code with those invalid values (allowing crashes or arbitrary memory clobbering). Since there's always a risk that later changes add unsafe sections to some parts of the implementation of for example BufWriter, it remains important to think about arithmetic wraparound / overflow.

@arjantop
Copy link
Contributor Author

That was just a statement after finding a problem until I fix it, not blaming you.

@arjantop
Copy link
Contributor Author

That build error just travis issues? @alexcrichton

@huonw
Copy link
Contributor

huonw commented Jul 12, 2014

@arjantop probably, I just restarted it. (Also, btw, bors the integration bot ignores travis results, i.e. it will do it's thing even if the travis build fails. e.g. this PR is still in the approved section of the queue, and will probably be tested in a few hours.)

bors added a commit that referenced this pull request Jul 12, 2014
First condition is not needed and just prevents 0 length writes

Fixes #15583
@bors bors closed this Jul 12, 2014
@bors bors merged commit 30f07e9 into rust-lang:master Jul 12, 2014
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 18, 2023
flip1995 pushed a commit to flip1995/rust that referenced this pull request Sep 4, 2025
The things being separated are sentences. So I think semicolons are the
right separators to use.

changelog: none
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.

Writing 0 bytes to a full buffer results in an error

6 participants