Fix temporary_cstring_as_ptr false negative

Fixes #4375.

Changes the check to test when `.unwrap().as_ptr()` is called on any
`Result<CString, _>` as suggested by @flip1995
(https://github.com/rust-lang/rust-clippy/issues/4375#issuecomment-520724123).
This commit is contained in:
Michael Wright 2019-08-21 07:35:04 +02:00
parent 460e2659f1
commit 59893bcab0
4 changed files with 45 additions and 15 deletions

View file

@ -8,7 +8,6 @@ use std::iter;
use if_chain::if_chain; use if_chain::if_chain;
use matches::matches; use matches::matches;
use rustc::hir; use rustc::hir;
use rustc::hir::def::{DefKind, Res};
use rustc::hir::intravisit::{self, Visitor}; use rustc::hir::intravisit::{self, Visitor};
use rustc::lint::{in_external_macro, LateContext, LateLintPass, Lint, LintArray, LintContext, LintPass}; use rustc::lint::{in_external_macro, LateContext, LateLintPass, Lint, LintArray, LintContext, LintPass};
use rustc::ty::{self, Predicate, Ty}; use rustc::ty::{self, Predicate, Ty};
@ -1668,13 +1667,12 @@ fn lint_extend(cx: &LateContext<'_, '_>, expr: &hir::Expr, args: &[hir::Expr]) {
} }
} }
fn lint_cstring_as_ptr(cx: &LateContext<'_, '_>, expr: &hir::Expr, new: &hir::Expr, unwrap: &hir::Expr) { fn lint_cstring_as_ptr(cx: &LateContext<'_, '_>, expr: &hir::Expr, source: &hir::Expr, unwrap: &hir::Expr) {
if_chain! { if_chain! {
if let hir::ExprKind::Call(ref fun, ref args) = new.node; let source_type = cx.tables.expr_ty(source);
if args.len() == 1; if let ty::Adt(def, substs) = source_type.sty;
if let hir::ExprKind::Path(ref path) = fun.node; if match_def_path(cx, def.did, &paths::RESULT);
if let Res::Def(DefKind::Method, did) = cx.tables.qpath_res(path, fun.hir_id); if match_type(cx, substs.type_at(0), &paths::CSTRING);
if match_def_path(cx, did, &paths::CSTRING_NEW);
then { then {
span_lint_and_then( span_lint_and_then(
cx, cx,

View file

@ -17,7 +17,7 @@ pub const CLONE_TRAIT_METHOD: [&str; 4] = ["core", "clone", "Clone", "clone"];
pub const CMP_MAX: [&str; 3] = ["core", "cmp", "max"]; pub const CMP_MAX: [&str; 3] = ["core", "cmp", "max"];
pub const CMP_MIN: [&str; 3] = ["core", "cmp", "min"]; pub const CMP_MIN: [&str; 3] = ["core", "cmp", "min"];
pub const COW: [&str; 3] = ["alloc", "borrow", "Cow"]; pub const COW: [&str; 3] = ["alloc", "borrow", "Cow"];
pub const CSTRING_NEW: [&str; 5] = ["std", "ffi", "c_str", "CString", "new"]; pub const CSTRING: [&str; 4] = ["std", "ffi", "c_str", "CString"];
pub const DEFAULT_TRAIT: [&str; 3] = ["core", "default", "Default"]; pub const DEFAULT_TRAIT: [&str; 3] = ["core", "default", "Default"];
pub const DEFAULT_TRAIT_METHOD: [&str; 4] = ["core", "default", "Default", "default"]; pub const DEFAULT_TRAIT_METHOD: [&str; 4] = ["core", "default", "Default", "default"];
pub const DEREF_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "Deref", "deref"]; pub const DEREF_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "Deref", "deref"];

View file

@ -1,9 +1,24 @@
#![deny(clippy::temporary_cstring_as_ptr)]
fn main() {} fn main() {}
#[allow(clippy::result_unwrap_used)]
fn temporary_cstring() { fn temporary_cstring() {
use std::ffi::CString; use std::ffi::CString;
CString::new("foo").unwrap().as_ptr(); CString::new("foo").unwrap().as_ptr();
CString::new("foo").expect("dummy").as_ptr(); CString::new("foo").expect("dummy").as_ptr();
} }
mod issue4375 {
use std::ffi::CString;
use std::os::raw::c_char;
extern "C" {
fn foo(data: *const c_char);
}
pub fn bar(v: &[u8]) {
let cstr = CString::new(v);
unsafe { foo(cstr.unwrap().as_ptr()) }
}
}

View file

@ -1,29 +1,46 @@
error: you are getting the inner pointer of a temporary `CString` error: you are getting the inner pointer of a temporary `CString`
--> $DIR/cstring.rs:7:5 --> $DIR/cstring.rs:8:5
| |
LL | CString::new("foo").unwrap().as_ptr(); LL | CString::new("foo").unwrap().as_ptr();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
= note: `#[deny(clippy::temporary_cstring_as_ptr)]` on by default note: lint level defined here
--> $DIR/cstring.rs:1:9
|
LL | #![deny(clippy::temporary_cstring_as_ptr)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: that pointer will be invalid outside this expression = note: that pointer will be invalid outside this expression
help: assign the `CString` to a variable to extend its lifetime help: assign the `CString` to a variable to extend its lifetime
--> $DIR/cstring.rs:7:5 --> $DIR/cstring.rs:8:5
| |
LL | CString::new("foo").unwrap().as_ptr(); LL | CString::new("foo").unwrap().as_ptr();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: you are getting the inner pointer of a temporary `CString` error: you are getting the inner pointer of a temporary `CString`
--> $DIR/cstring.rs:8:5 --> $DIR/cstring.rs:9:5
| |
LL | CString::new("foo").expect("dummy").as_ptr(); LL | CString::new("foo").expect("dummy").as_ptr();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
= note: that pointer will be invalid outside this expression = note: that pointer will be invalid outside this expression
help: assign the `CString` to a variable to extend its lifetime help: assign the `CString` to a variable to extend its lifetime
--> $DIR/cstring.rs:8:5 --> $DIR/cstring.rs:9:5
| |
LL | CString::new("foo").expect("dummy").as_ptr(); LL | CString::new("foo").expect("dummy").as_ptr();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 2 previous errors error: you are getting the inner pointer of a temporary `CString`
--> $DIR/cstring.rs:22:22
|
LL | unsafe { foo(cstr.unwrap().as_ptr()) }
| ^^^^^^^^^^^^^^^^^^^^^^
|
= note: that pointer will be invalid outside this expression
help: assign the `CString` to a variable to extend its lifetime
--> $DIR/cstring.rs:22:22
|
LL | unsafe { foo(cstr.unwrap().as_ptr()) }
| ^^^^^^^^^^^^^
error: aborting due to 3 previous errors