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

libc ANSI functions must be avoided on Windows #9822

Closed
klutzy opened this issue Oct 12, 2013 · 2 comments
Closed

libc ANSI functions must be avoided on Windows #9822

klutzy opened this issue Oct 12, 2013 · 2 comments
Labels
O-windows Operating system: Windows

Comments

@klutzy
Copy link
Contributor

klutzy commented Oct 12, 2013

Some libc functions e.g. fopen accept or return char*. However, on Windows, they don't understand utf-8, instead they speak ANSI.
If the function returns char* which contains non-ascii, utf-8 assertion error rises. See #9418 and #9618 for actual cases. (They may be fixed by #9812)
If the function accepts char* parameter but we put non-ascii utf8, then the api fails to understand it thus returns error.

To fix it, we must avoid them and use UTF-16 alternatives e.g. _wfopen on Windows.
This requires converting utf8 into utf16 before calling libc apis.
Similarly, WinAPI with A postfix uses ANSI. We must use W functions instead.

Here is list of problematic points I know (far from complete):

  • std::io uses fopen. (I don't think we should fix it now since it will be replaced by std::rt::io; libuv accepts utf8 and internally calls W-apis so it is safe)
  • std::os::env uses GetEnvironmentStringsA and FreeEnvironmentStringsA.
  • std::os::rename_file uses libc::rename. This will fail to rename ☆.txt to ★.txt on Windows.
@nitric1
Copy link
Contributor

nitric1 commented Oct 21, 2013

  • std::libc::types::os::arch::extra::{LPCTSTR, LPTSTR, LPTCH} must be removed (TCHAR and its family are just for compatibility with ANSI, so not required by rust std.)
  • std::rt::io::native::process::spawn_process_os (called by std::rt::io::native::process::Process::new) uses CreateProcessA. It must be changed to W (and also STARTUPINFO.)

@alexcrichton
Copy link
Member

Closing, most functions are no longer using their ANSI equivalents, I think the only remaining use case is #13815.

More specific follow-up issues can be opened.

flip1995 pushed a commit to flip1995/rust that referenced this issue Nov 21, 2022
Add `unnecessary_safety_doc` lint

changelog: [`unnecessary_safety_doc`]: Add `unnecessary_safety_doc` lint

fixes rust-lang/rust-clippy#6880

This lint does not trigger for private functions, just like `missing_safety_docs`. Reason for that was implementation simplicity and because I figured asking first would make more sense, so if it should trigger for private functions as well let me know and I'll fix that up as well.
flip1995 pushed a commit to flip1995/rust that referenced this issue Dec 1, 2022
…ip1995

Move `unnecessary_unsafety_doc` to `pedantic`

This lint was added in rust-lang#9822. I like the idea, but also agree with rust-lang#9986 as well. I think it should at least not be warn-by-default. This is one of these cases, where I'd like a group between pedantic and restriction. But I believe that users using `#![warn(clippy::pedantic)]` will know how to enable the lint if they disagree with it.

---

Since the lint is new:

changelog: none

r? `@flip1995` since I'd suggest back porting this, the original PR was merged 16 days ago.

Closes: rust-lang#9986 (While it doesn't address everything, I believe that this is the best compromise)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-windows Operating system: Windows
Projects
None yet
Development

No branches or pull requests

3 participants