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.

Sign up for GitHub

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

Windows: Use WriteFile to write to a UTF-8 console #134622

Merged
merged 1 commit into from
Dec 27, 2024
+26 −1
Merged

Windows: Use WriteFile to write to a UTF-8 console #134622

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions library/std/src/sys/pal/windows/c/bindings.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2425,6 +2425,7 @@ Windows.Win32.System.Console.ENABLE_VIRTUAL_TERMINAL_PROCESSING
Windows.Win32.System.Console.ENABLE_WINDOW_INPUT
Windows.Win32.System.Console.ENABLE_WRAP_AT_EOL_OUTPUT
Windows.Win32.System.Console.GetConsoleMode
Windows.Win32.System.Console.GetConsoleOutputCP
Windows.Win32.System.Console.GetStdHandle
Windows.Win32.System.Console.ReadConsoleW
Windows.Win32.System.Console.STD_ERROR_HANDLE
Expand Down
2 changes: 2 additions & 0 deletions library/std/src/sys/pal/windows/c/windows_sys.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ windows_targets::link!("kernel32.dll" "system" fn FreeEnvironmentStringsW(penv :
windows_targets::link!("kernel32.dll" "system" fn GetActiveProcessorCount(groupnumber : u16) -> u32);
windows_targets::link!("kernel32.dll" "system" fn GetCommandLineW() -> PCWSTR);
windows_targets::link!("kernel32.dll" "system" fn GetConsoleMode(hconsolehandle : HANDLE, lpmode : *mut CONSOLE_MODE) -> BOOL);
windows_targets::link!("kernel32.dll" "system" fn GetConsoleOutputCP() -> u32);
windows_targets::link!("kernel32.dll" "system" fn GetCurrentDirectoryW(nbufferlength : u32, lpbuffer : PWSTR) -> u32);
windows_targets::link!("kernel32.dll" "system" fn GetCurrentProcess() -> HANDLE);
windows_targets::link!("kernel32.dll" "system" fn GetCurrentProcessId() -> u32);
Expand Down Expand Up @@ -3317,6 +3318,7 @@ pub struct XSAVE_FORMAT {
pub XmmRegisters: [M128A; 8],
pub Reserved4: [u8; 224],
}

#[cfg(target_arch = "arm")]
#[repr(C)]
pub struct WSADATA {
Expand Down
24 changes: 23 additions & 1 deletion library/std/src/sys/pal/windows/stdio.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,21 +84,43 @@ fn is_console(handle: c::HANDLE) -> bool {
unsafe { c::GetConsoleMode(handle, &mut mode) != 0 }
}

/// Returns true if the attached console's code page is currently UTF-8.
#[cfg(not(target_vendor = "win7"))]
fn is_utf8_console() -> bool {
unsafe { c::GetConsoleOutputCP() == c::CP_UTF8 }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this cheap enough to call on every write? I guess we're already calling GetConsoleMode on every write...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I don't love it but it is cheap. Especially when compared to the previous code where if is_console is true then it will be doing a full UTF-8 to UTF-16 conversion using a small buffer (necessitating multiple calls to write).

}

#[cfg(target_vendor = "win7")]
fn is_utf8_console() -> bool {
// Windows 7 has a fun "feature" where WriteFile on a console handle will return
// the number of UTF-16 code units written and not the number of bytes from the input string.
// So we always claim the console isn't UTF-8 to trigger the WriteConsole fallback code.
false
}

fn write(handle_id: u32, data: &[u8], incomplete_utf8: &mut IncompleteUtf8) -> io::Result<usize> {
if data.is_empty() {
return Ok(0);
}

let handle = get_handle(handle_id)?;
if !is_console(handle) {
if !is_console(handle) || is_utf8_console() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be worried about race conditions at all with changes in the mode setting? My sense is probably no?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not. It is possible that the code page could be changed after is_utf8_console and before the write. But changing the console code page is like changing any global state (e.g. an environment variable or the current directory). That is, it's the sort of thing you maybe set once on startup and don't touch again (unless you're very careful).

unsafe {
let handle = Handle::from_raw_handle(handle);
let ret = handle.write(data);
let _ = handle.into_raw_handle(); // Don't close the handle
return ret;
}
} else {
write_console_utf16(data, incomplete_utf8, handle)
}
}

fn write_console_utf16(
data: &[u8],
incomplete_utf8: &mut IncompleteUtf8,
handle: c::HANDLE,
) -> io::Result<usize> {
if incomplete_utf8.len > 0 {
assert!(
incomplete_utf8.len < 4,
Expand Down
Loading