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

Lint against String::from_raw_parts #14293

Open
robertbastian opened this issue Feb 25, 2025 · 4 comments
Open

Lint against String::from_raw_parts #14293

robertbastian opened this issue Feb 25, 2025 · 4 comments
Labels
A-lint Area: New lints

Comments

@robertbastian
Copy link
Contributor

robertbastian commented Feb 25, 2025

What it does

Lints against usages of String::from_raw_parts. This should be done in two steps, Vec::from_raw_parts and String::from_utf8_unchecked.

rust-lang/rust#136775 updated String::from_raw_parts's docs to delegate the safety requirements to Vec::from_raw_parts's and String::from_utf8_unchecked's docs anyway.

Advantage

  • String::from_raw_parts has conflated safety requirements that should be justified in two steps:
    • Vec::from_raw_parts has complicated language-level safety requirements that have to be manually justified and are insta-UB (?) if violated
    • String::from_utf8_unchecked has a relatively trivial library-level safety requirement, that could be checked by using String::from_utf8 instead

Drawbacks

No response

Example

// SAFETY: 
// * `ptr` was allocated using the global allocator
// * `T` (= `u8`) has the same alignment as what `ptr` was allocated with
// * The size of T (= u8) times the capacity is the same size as `ptr` was allocated with
// * `len` is use for both `len` and `capacity`
// * The first `len` values are properly initialized values of type `T` (= `u8`).
// * `capacity` (= `len`) is the capacity that `ptr` was allocated with.
// * The allocated size in bytes is no larger than `isize::MAX`
// * The bytes contain valid UTF-8
let string = unsafe { String::from_raw_parts(pointer, len) };

Could be written as:

// SAFETY: 
// * `ptr` was allocated using the global allocator
// * `T` (= `u8`) has the same alignment as what `ptr` was allocated with
// * The size of T (= u8) times the capacity is the same size as `ptr` was allocated with
// * `len` is use for both `len` and `capacity`
// * The first `len` values are properly initialized values of type `T` (= `u8`).
// * `capacity` (= `len`) is the capacity that `ptr` was allocated with.
// * The allocated size in bytes is no larger than `isize::MAX`
let bytes = unsafe { Vec::from_raw_parts(ptr, len, len) };
// SAFETY: 
// * The bytes contain valid UTF-8
let string = unsafe { String::from_utf8_unchecked(bytes) };
@robertbastian robertbastian added the A-lint Area: New lints label Feb 25, 2025
@samueltardieu
Copy link
Contributor

Note that you can already warn about or deny using String::from_raw_parts() by using the clippy::disallowed_methods lint. You can even indicate the reason for disallowing it.

@robertbastian
Copy link
Contributor Author

I'm aware. I'm not the only one with this concern: rust-lang/libs-team#167 (comment)

@samueltardieu
Copy link
Contributor

So what do you propose exactly? Adding a new dedicated restriction lint?

@robertbastian
Copy link
Contributor Author

Yes a new lint, could be pedantic, style, restriction, suspicious, idk.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints
Projects
None yet
Development

No branches or pull requests

2 participants