Partially revert #107200

[joshtriplett]

Ok(0) is indeed something the caller may interpret as an error, but
that's the correct thing to return if the writer can't accept any more
bytes.

[rustbot]

r? @m-ou-se

(rustbot has picked a reviewer for you, use r? to override)

[mina86]

but that's the correct thing to return if the writer can't accept any more bytes.

I have no voting power but nonetheless I’m strongly against such API.

[joshtriplett]

@mina86 It's the current stable API in std's own implementations of Write (such as for &mut [u8]), and it seems to be what at least some callers of write expect. As identified in #107200 (comment) it seems like it'd be a breaking change for std's own implementations of Write to change this behavior.

[mina86]

$ cat src/main.rs
use std::io::Write;

fn main() -> std::io::Result<()> {
    let buf = [0; 1024 * 1024];
    let mut file = std::fs::File::create("/mnt/foo")?;
    loop {
        eprintln!("{}", file.write(&buf)?);
    }
}
$ cargo run
    Finished dev [unoptimized] target(s) in 0.00s
     Running `target/debug/a`
913408
Error: Os { code: 28, kind: StorageFull, message: "No space left on device" }

No matter how you slice it, std is inconsistent. With StorageFull finally being added, we can change the documentation to encourage its use and consistency in future code.

[joshtriplett]

StorageFull is being used correctly there: it's returned by the OS for a full disk, and Rust passes it along to the caller.

[m-ou-se]

@mina86 We're not reverting this to say that write should be returning Ok(0) for "out of storage", but because there already is plenty of code that returns Ok(0) for EOF (including the raw OS APIs), which a caller should be able to handle.

That doesn't mean that the resulting wording in the docs here is final. We might still want to add a note that recommends using a StorageFull error in certain cases, for example. (But that's something for after the 1.72 release. And probably only when ErrorKind::StorageFull itself is stable.)

@bors r+

[bors]

📌 Commit 5210f48 has been approved by m-ou-se

It is now in the queue for this repository.

[bors]

⌛ Testing commit 5210f48 with merge a9270420c3e25469f73da2c3a5a4db03c6d286bc...

[bors]

☀️ Test successful - checks-actions
Approved by: m-ou-se
Pushing a9270420c3e25469f73da2c3a5a4db03c6d286bc to master...

[bors]

👀 Test was successful, but fast-forwarding failed: 422 Update is not a fast forward

[cuviper]

@bors retry

[rust-timer]

Finished benchmarking commit (a9270420c3e25469f73da2c3a5a4db03c6d286bc): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.4%	[0.4%, 0.4%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.4%	[0.4%, 0.4%]	1

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.3%	[-3.8%, -2.3%]	3
All ❌✅ (primary)	-	-	0

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 634.954s -> 633.803s (-0.18%)
Artifact size: 347.10 MiB -> 347.16 MiB (0.02%)

[cuviper]

@bors rollup