Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
Box::new(|_| Box::<unused_async::UnusedAsync>::default()),
Box::new(move |tcx| Box::new(disallowed_types::DisallowedTypes::new(tcx, conf))),
Box::new(move |tcx| Box::new(missing_enforced_import_rename::ImportRename::new(tcx, conf))),
Box::new(|_| Box::new(strlen_on_c_strings::StrlenOnCStrings)),
Box::new(move |_| Box::new(strlen_on_c_strings::StrlenOnCStrings::new(conf))),
Box::new(move |_| Box::new(self_named_constructors::SelfNamedConstructors)),
Box::new(move |_| Box::new(iter_not_returning_iterator::IterNotReturningIterator)),
Box::new(move |_| Box::new(manual_assert::ManualAssert)),
Expand Down
68 changes: 45 additions & 23 deletions clippy_lints/src/strlen_on_c_strings.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
use clippy_config::Conf;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::msrvs::{self, Msrv};
use clippy_utils::res::MaybeDef;
use clippy_utils::source::snippet_with_context;
use clippy_utils::visitors::is_expr_unsafe;
use clippy_utils::{match_libc_symbol, sym};
use rustc_errors::Applicability;
use rustc_hir::{Block, BlockCheckMode, Expr, ExprKind, LangItem, Node, UnsafeSource};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::declare_lint_pass;
use rustc_session::impl_lint_pass;

declare_clippy_lint! {
/// ### What it does
/// Checks for usage of `libc::strlen` on a `CString` or `CStr` value,
/// and suggest calling `as_bytes().len()` or `to_bytes().len()` respectively instead.
/// and suggest calling `count_bytes()`instead.
///
/// ### Why is this bad?
/// libc::strlen is an unsafe function, which we don't need to call
Expand All @@ -27,17 +29,26 @@ declare_clippy_lint! {
/// ```rust, no_run
/// use std::ffi::CString;
/// let cstring = CString::new("foo").expect("CString::new failed");
/// let len = cstring.as_bytes().len();
/// let len = cstring.count_bytes();
/// ```
#[clippy::version = "1.55.0"]
pub STRLEN_ON_C_STRINGS,
complexity,
"using `libc::strlen` on a `CString` or `CStr` value, while `as_bytes().len()` or `to_bytes().len()` respectively can be used instead"
"using `libc::strlen` on a `CString` or `CStr` value, while `count_bytes()` can be used instead"
}

declare_lint_pass!(StrlenOnCStrings => [STRLEN_ON_C_STRINGS]);
pub struct StrlenOnCStrings<'a> {
msrv: &'a Msrv,
}
impl<'a> StrlenOnCStrings<'a> {
pub fn new(conf: &'a Conf) -> Self {
Self { msrv: &conf.msrv }
}
}

impl_lint_pass!(StrlenOnCStrings<'_> => [STRLEN_ON_C_STRINGS]);

impl<'tcx> LateLintPass<'tcx> for StrlenOnCStrings {
impl<'tcx> LateLintPass<'tcx> for StrlenOnCStrings<'_> {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if !expr.span.from_expansion()
&& let ExprKind::Call(func, [recv]) = expr.kind
Expand All @@ -58,26 +69,37 @@ impl<'tcx> LateLintPass<'tcx> for StrlenOnCStrings {
_ => expr.span,
};

let ty = cx.typeck_results().expr_ty(self_arg).peel_refs();
let mut app = Applicability::MachineApplicable;
let val_name = snippet_with_context(cx, self_arg.span, ctxt, "..", &mut app).0;
let method_name = if ty.is_diag_item(cx, sym::cstring_type) {
"as_bytes"
} else if ty.is_lang_item(cx, LangItem::CStr) {
"to_bytes"
if self.msrv.meets(cx, msrvs::CSTR_COUNT_BYTES) {
span_lint_and_sugg(
cx,
STRLEN_ON_C_STRINGS,
span,
"using `libc::strlen` on a `CString` or `CStr` value",
"try",
format!("{val_name}.count_bytes()"),
app,
);
} else {
return;
};

span_lint_and_sugg(
cx,
STRLEN_ON_C_STRINGS,
span,
"using `libc::strlen` on a `CString` or `CStr` value",
"try",
format!("{val_name}.{method_name}().len()"),
app,
);
let ty = cx.typeck_results().expr_ty(self_arg).peel_refs();
let method_name = if ty.is_diag_item(cx, sym::cstring_type) {
"as_bytes"
} else if ty.is_lang_item(cx, LangItem::CStr) {
"to_bytes"
} else {
return;
};
span_lint_and_sugg(
cx,
STRLEN_ON_C_STRINGS,
span,
"using `libc::strlen` on a `CString` or `CStr` value",
"try",
format!("{val_name}.{method_name}().len()"),
app,
);
}
}
}
}
2 changes: 1 addition & 1 deletion clippy_utils/src/msrvs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ msrv_aliases! {
1,82,0 { IS_NONE_OR, REPEAT_N, RAW_REF_OP, SPECIALIZED_TO_STRING_FOR_REFS }
1,81,0 { LINT_REASONS_STABILIZATION, ERROR_IN_CORE, EXPLICIT_SELF_TYPE_ELISION, DURATION_ABS_DIFF }
1,80,0 { BOX_INTO_ITER, LAZY_CELL }
1,79,0 { CONST_BLOCKS }
1,79,0 { CONST_BLOCKS, CSTR_COUNT_BYTES }
1,77,0 { C_STR_LITERALS }
1,76,0 { PTR_FROM_REF, OPTION_RESULT_INSPECT }
1,75,0 { OPTION_AS_SLICE }
Expand Down
36 changes: 29 additions & 7 deletions tests/ui/strlen_on_c_strings.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -8,30 +8,52 @@ use std::ffi::{CStr, CString};
fn main() {
// CString
let cstring = CString::new("foo").expect("CString::new failed");
let _ = cstring.as_bytes().len();
let _ = cstring.count_bytes();
//~^ strlen_on_c_strings

// CStr
let cstr = CStr::from_bytes_with_nul(b"foo\0").expect("CStr::from_bytes_with_nul failed");
let _ = cstr.to_bytes().len();
let _ = cstr.count_bytes();
//~^ strlen_on_c_strings

let _ = cstr.to_bytes().len();
let _ = cstr.count_bytes();
//~^ strlen_on_c_strings

let pcstr: *const &CStr = &cstr;
let _ = unsafe { (*pcstr).to_bytes().len() };
let _ = unsafe { (*pcstr).count_bytes() };
//~^ strlen_on_c_strings

unsafe fn unsafe_identity<T>(x: T) -> T {
x
}
let _ = unsafe { unsafe_identity(cstr).to_bytes().len() };
let _ = unsafe { unsafe_identity(cstr).count_bytes() };
//~^ strlen_on_c_strings
let _ = unsafe { unsafe_identity(cstr) }.to_bytes().len();
let _ = unsafe { unsafe_identity(cstr) }.count_bytes();
//~^ strlen_on_c_strings

let f: unsafe fn(_) -> _ = unsafe_identity;
let _ = unsafe { f(cstr).to_bytes().len() };
let _ = unsafe { f(cstr).count_bytes() };
//~^ strlen_on_c_strings
}

#[clippy::msrv = "1.78"]
fn msrv_1_78() {
let cstr = CStr::from_bytes_with_nul(b"foo\0").expect("CStr::from_bytes_with_nul failed");
let _ = cstr.to_bytes().len();
//~^ strlen_on_c_strings

let cstring = CString::new("foo").expect("CString::new failed");
let _ = cstring.as_bytes().len();
//~^ strlen_on_c_strings
}

#[clippy::msrv = "1.79"]
fn msrv_1_79() {
let cstr = CStr::from_bytes_with_nul(b"foo\0").expect("CStr::from_bytes_with_nul failed");
let _ = cstr.count_bytes();
//~^ strlen_on_c_strings

let cstring = CString::new("foo").expect("CString::new failed");
let _ = cstring.count_bytes();
//~^ strlen_on_c_strings
}
22 changes: 22 additions & 0 deletions tests/ui/strlen_on_c_strings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,25 @@ fn main() {
let _ = unsafe { strlen(f(cstr).as_ptr()) };
//~^ strlen_on_c_strings
}

#[clippy::msrv = "1.78"]
fn msrv_1_78() {
let cstr = CStr::from_bytes_with_nul(b"foo\0").expect("CStr::from_bytes_with_nul failed");
let _ = unsafe { libc::strlen(cstr.as_ptr()) };
//~^ strlen_on_c_strings

let cstring = CString::new("foo").expect("CString::new failed");
let _ = unsafe { libc::strlen(cstring.as_ptr()) };
//~^ strlen_on_c_strings
}

#[clippy::msrv = "1.79"]
fn msrv_1_79() {
let cstr = CStr::from_bytes_with_nul(b"foo\0").expect("CStr::from_bytes_with_nul failed");
let _ = unsafe { libc::strlen(cstr.as_ptr()) };
//~^ strlen_on_c_strings

let cstring = CString::new("foo").expect("CString::new failed");
let _ = unsafe { libc::strlen(cstring.as_ptr()) };
//~^ strlen_on_c_strings
}
40 changes: 32 additions & 8 deletions tests/ui/strlen_on_c_strings.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error: using `libc::strlen` on a `CString` or `CStr` value
--> tests/ui/strlen_on_c_strings.rs:11:13
|
LL | let _ = unsafe { libc::strlen(cstring.as_ptr()) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `cstring.as_bytes().len()`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `cstring.count_bytes()`
|
= note: `-D clippy::strlen-on-c-strings` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::strlen_on_c_strings)]`
Expand All @@ -11,37 +11,61 @@ error: using `libc::strlen` on a `CString` or `CStr` value
--> tests/ui/strlen_on_c_strings.rs:16:13
|
LL | let _ = unsafe { libc::strlen(cstr.as_ptr()) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `cstr.to_bytes().len()`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `cstr.count_bytes()`

error: using `libc::strlen` on a `CString` or `CStr` value
--> tests/ui/strlen_on_c_strings.rs:19:13
|
LL | let _ = unsafe { strlen(cstr.as_ptr()) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `cstr.to_bytes().len()`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `cstr.count_bytes()`

error: using `libc::strlen` on a `CString` or `CStr` value
--> tests/ui/strlen_on_c_strings.rs:23:22
|
LL | let _ = unsafe { strlen((*pcstr).as_ptr()) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(*pcstr).to_bytes().len()`
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(*pcstr).count_bytes()`

error: using `libc::strlen` on a `CString` or `CStr` value
--> tests/ui/strlen_on_c_strings.rs:29:22
|
LL | let _ = unsafe { strlen(unsafe_identity(cstr).as_ptr()) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unsafe_identity(cstr).to_bytes().len()`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unsafe_identity(cstr).count_bytes()`

error: using `libc::strlen` on a `CString` or `CStr` value
--> tests/ui/strlen_on_c_strings.rs:31:13
|
LL | let _ = unsafe { strlen(unsafe { unsafe_identity(cstr) }.as_ptr()) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unsafe { unsafe_identity(cstr) }.to_bytes().len()`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unsafe { unsafe_identity(cstr) }.count_bytes()`

error: using `libc::strlen` on a `CString` or `CStr` value
--> tests/ui/strlen_on_c_strings.rs:35:22
|
LL | let _ = unsafe { strlen(f(cstr).as_ptr()) };
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `f(cstr).to_bytes().len()`
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `f(cstr).count_bytes()`

error: using `libc::strlen` on a `CString` or `CStr` value
--> tests/ui/strlen_on_c_strings.rs:42:13
|
LL | let _ = unsafe { libc::strlen(cstr.as_ptr()) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `cstr.to_bytes().len()`

error: using `libc::strlen` on a `CString` or `CStr` value
--> tests/ui/strlen_on_c_strings.rs:46:13
|
LL | let _ = unsafe { libc::strlen(cstring.as_ptr()) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `cstring.as_bytes().len()`

error: using `libc::strlen` on a `CString` or `CStr` value
--> tests/ui/strlen_on_c_strings.rs:53:13
|
LL | let _ = unsafe { libc::strlen(cstr.as_ptr()) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `cstr.count_bytes()`

error: using `libc::strlen` on a `CString` or `CStr` value
--> tests/ui/strlen_on_c_strings.rs:57:13
|
LL | let _ = unsafe { libc::strlen(cstring.as_ptr()) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `cstring.count_bytes()`

error: aborting due to 7 previous errors
error: aborting due to 11 previous errors