-
Notifications
You must be signed in to change notification settings - Fork 14.1k
fix(lib-std-fs): handle usize overflow in read*
#143462
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
Conversation
This comment has been minimized.
This comment has been minimized.
|
Shouldn't this fail with Also modifications to |
usize overflow in read_to_stringusize overflow in read*
|
Hi, is there any particular reason you have not added regression tests for this fix? Regression tests are useful for many reasons, for example:
|
|
Thanks for the reminder! I do agree that tests would be a good idea. I'll have to re-read the contrib-docs, because IDK how to do it properly |
|
I don't think a test here is necessary. The previous behaviour wasn't a bug, just a suboptimal implementation. And you'd probably have to track the amount of read syscalls in order to test this, which is way to overkill for such a simple fix. @bors r+ |
Rollup of 5 pull requests Successful merges: - #143462 (fix(lib-std-fs): handle `usize` overflow in `read*`) - #144651 (Implementation: `#[feature(nonpoison_condvar)]`) - #145465 (Stabilize `array_repeat` feature) - #145776 (Optimize `.ilog({2,10})` to `.ilog{2,10}()`) - #145969 (Add Duration::from_nanos_u128) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #143462 - Rudxain:read_to_string_usize, r=joboet fix(lib-std-fs): handle `usize` overflow in `read*` I assume this is a non-breaking change, as there would be an OOM `panic` anyways. This patch ensures a fast-fail when there's not enough memory to load the file. This only changes behavior on platforms where `usize` is smaller than 64bits
…boet fix(lib-std-fs): handle `usize` overflow in `read*` I assume this is a non-breaking change, as there would be an OOM `panic` anyways. This patch ensures a fast-fail when there's not enough memory to load the file. This only changes behavior on platforms where `usize` is smaller than 64bits
I assume this is a non-breaking change, as there would be an OOM
panicanyways. This patch ensures a fast-fail when there's not enough memory to load the file. This only changes behavior on platforms whereusizeis smaller than 64bits