Fixup stout/stderr on Windows #32184
Conversation
WriteConsoleW can fail if called with a large buffer so we need to slice any stdout/stderr output. However the current slicing has a few problems: 1. It slices by byte but still expects valid UTF-8. 2. The slicing happens even when not outputting to a console. 3. panic! output is not sliced. This fixes these issues by moving the slice to right before WriteConsoleW and slicing on a char boundary.
| // [1]: https://tahoe-lafs.org/trac/tahoe-lafs/ticket/1232 | ||
| // [2]: http://www.mail-archive.com/[email protected]/msg00661.html | ||
| const OUT_MAX: usize = 8192; | ||
| let data_len; |
There was a problem hiding this comment.
This could also be done by returning a tuple from the match block.
|
Looks good to me. cc #23344 |
| // [1]: https://tahoe-lafs.org/trac/tahoe-lafs/ticket/1232 | ||
| // [2]: http://www.mail-archive.com/[email protected]/msg00661.html | ||
| const OUT_MAX: usize = 8192; | ||
| let (utf16, data_len) = match str::from_utf8(data).ok() { |
There was a problem hiding this comment.
Perhaps this could do something like:
let len = cmp::min(data.len(), OUT_MAX);
let data = match str::from_utf8(&data[..len]) {
Ok(s) => s,
Err(ref e) if e.valid_up_to() == 0 => return Err(..),
Err(e) => str::from_utf8(&s[..e.valid_up_to()]).unwrap(),
};
let data = ...;There was a problem hiding this comment.
In the sense that it would:
- Be the same performance for the "fast path"
- Handle writing partially utf-8 data naturally
- Not have to encode a let's-go-backwards loop
|
Can you add a test for this as well? The test in #30399 I think should be good to add. |
This makes it output as much valid UTF-8 as it can then return failure.
|
I've applied the suggested code changes however I'm not sure where to add a test as it will only make sense if output to a real console which isn't the case for run-pass for example. |
|
Technically the test could |
|
Yeah I've tested and things like the following now work as expected: fn main () {
let v = vec![255u8; 5000];
let s = String::from_utf8_lossy(&v);
println!("{}", s);
}fn main() {
let s = unsafe { String::from_utf8_unchecked(vec![b'a'; 1 << 32]) };
panic!("{}", s);
} |
Fixup stout/stderr on Windows WriteConsoleW can fail if called with a large buffer so we need to slice any stdout/stderr output. However the current slicing has a few problems: 1. It slices by byte but still expects valid UTF-8. 2. The slicing happens even when not outputting to a console. 3. panic! output is not sliced. This fixes these issues by moving the slice to right before WriteConsoleW and slicing on a char boundary.
|
Seems like bors messed up and didn't merge this? @alexcrichton |
|
Looks like bors did indeed merge but github didn't detect it was merged, ah well. |
WriteConsoleW can fail if called with a large buffer so we need to slice
any stdout/stderr output.
However the current slicing has a few problems:
This fixes these issues by moving the slice to right before
WriteConsoleW and slicing on a char boundary.