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

SeekFrom::End should take a non-positive number #10799

Open
drewtato opened this issue May 18, 2023 · 1 comment
Open

SeekFrom::End should take a non-positive number #10799

drewtato opened this issue May 18, 2023 · 1 comment
Assignees
Labels
A-lint Area: New lints

Comments

@drewtato
Copy link

drewtato commented May 18, 2023

What it does

Notices possible typos of passing a positive literal number to the std::io::SeekFrom::End variant, which is almost always supposed to be used with negative numbers or zero.

Other Seek lints: #7886 seek_from_current, #8600 seek_to_start_instead_of_rewind

Lint Name

seek_from_end_positive

Category

suspicious

Advantage

This is almost certainly a typo. When used with std::fs::File, this results in an empty implementer of std::io::Read, which is the same std::io::Read behavior as SeekFrom::End(0). It is likely the programmer intended to use the negative version of the number.

Although rare, there is no guarantee that the implementer will remember how far past the end it has seeked, leading to possibly inconsistent behavior when operating generically over Seek implementations. A seek past the end could return the stream length, or u32::MAX, or anything else depending on the implementation details.

The main implication of SeekFrom::End is that there is nothing beyond zero, so seeking past the end can still be confusing even when done correctly.

Drawbacks

It is possible SeekFrom is being used outside of Seek::seek. It could also be storing a value that will be modified before passing to Seek::seek. These are uncommon.

It is possible that an implementer of Seek would have meaningful behavior when seeked past the end, especially if this is paired with another (likely variable) seek that uses a negative SeekFrom::Current to bring it back into the valid range of the implementer. However, those are often combined into one Seek::seek call anyway.

Example

use std::io::{Cursor, Seek, SeekFrom};
let mut buf = Cursor::new(vec![1, 2, 3]);
buf.seek(SeekFrom::End(1)).unwrap();

Should be written as:

use std::io::{Cursor, Seek, SeekFrom};
let mut buf = Cursor::new(vec![1, 2, 3]);
buf.seek(SeekFrom::End(-1)).unwrap();
@drewtato drewtato added the A-lint Area: New lints label May 18, 2023
@anthonygedeon
Copy link

@rustbot claim

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints
Projects
None yet
Development

No branches or pull requests

2 participants