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 on libc::strlen(some_cstr.as_ptr()) #7145

Closed
joshtriplett opened this issue Apr 29, 2021 · 4 comments · Fixed by #7243
Closed

lint on libc::strlen(some_cstr.as_ptr()) #7145

joshtriplett opened this issue Apr 29, 2021 · 4 comments · Fixed by #7243
Assignees
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy

Comments

@joshtriplett
Copy link
Member

joshtriplett commented Apr 29, 2021

What it does

This lint would flag the use of libc::strlen on a CString or CStr value, and suggest calling to_bytes().len() instead.

Categories (optional)

  • Kind: clippy::complexity

What is the advantage of the recommended code over the original code

This avoids calling an unsafe libc function. Currently, it also avoids calculating the length; in the future, this may become equivalent to calling strlen at runtime, but it'll still avoid the need for an unsafe libc call.

Drawbacks

None.

Example

libc::strlen(name.as_ptr())

Could be written as:

name.to_bytes().len()
@joshtriplett joshtriplett added the A-lint Area: New lints label Apr 29, 2021
@joshtriplett
Copy link
Member Author

Inspired by rust-lang/rust#84589 (comment)

@giraffate giraffate added the good-first-issue These issues are a good way to get started with Clippy label Apr 30, 2021
@mgacek8
Copy link
Contributor

mgacek8 commented May 16, 2021

What about for CString suggesting as_bytes().len() instead of to_bytes().len() ?
as_bytes() shouldn't calculate the length in the future.

@joshtriplett
Copy link
Member Author

@mgacek8 That seems reasonable, yes. The lint should apply to either CStr or CString; the suggested replacement can be different for the two.

@mgacek8
Copy link
Contributor

mgacek8 commented May 18, 2021

I'll give it a shot.
@rustbot claim

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 19, 2021
…_type, r=davidtwco

Add diagnostic item to `CStr`

Required for clippy issue: rust-lang/rust-clippy#7145
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 19, 2021
…_type, r=davidtwco

Add diagnostic item to `CStr`

Required for clippy issue: rust-lang/rust-clippy#7145
RalfJung added a commit to RalfJung/rust that referenced this issue May 19, 2021
…_type, r=davidtwco

Add diagnostic item to `CStr`

Required for clippy issue: rust-lang/rust-clippy#7145
@bors bors closed this as completed in 64d74df Jul 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants