Add a TEMPORARY_CSTRING_AS_PTR lint

This commit is contained in:
mcarton 2016-04-14 17:25:31 +02:00
parent 831b8fc1b5
commit 1789430a49
5 changed files with 68 additions and 5 deletions

View file

@ -14,7 +14,7 @@ Table of contents:
* [License](#license)
##Lints
There are 140 lints included in this crate:
There are 141 lints included in this crate:
name | default | meaning
---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@ -135,6 +135,7 @@ name
[suspicious_assignment_formatting](https://github.com/Manishearth/rust-clippy/wiki#suspicious_assignment_formatting) | warn | suspicious formatting of `*=`, `-=` or `!=`
[suspicious_else_formatting](https://github.com/Manishearth/rust-clippy/wiki#suspicious_else_formatting) | warn | suspicious formatting of `else if`
[temporary_assignment](https://github.com/Manishearth/rust-clippy/wiki#temporary_assignment) | warn | assignments to temporaries
[temporary_cstring_as_ptr](https://github.com/Manishearth/rust-clippy/wiki#temporary_cstring_as_ptr) | warn | getting the inner pointer of a temporary `CString`
[too_many_arguments](https://github.com/Manishearth/rust-clippy/wiki#too_many_arguments) | warn | functions with too many arguments
[toplevel_ref_arg](https://github.com/Manishearth/rust-clippy/wiki#toplevel_ref_arg) | warn | An entire binding was declared as `ref`, in a function argument (`fn foo(ref x: Bar)`), or a `let` statement (`let ref x = foo()`). In such cases, it is preferred to take references with `&`.
[transmute_ptr_to_ref](https://github.com/Manishearth/rust-clippy/wiki#transmute_ptr_to_ref) | warn | transmutes from a pointer to a reference type

View file

@ -325,6 +325,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
methods::SEARCH_IS_SOME,
methods::SHOULD_IMPLEMENT_TRAIT,
methods::SINGLE_CHAR_PATTERN,
methods::TEMPORARY_CSTRING_AS_PTR,
methods::WRONG_SELF_CONVENTION,
minmax::MIN_MAX,
misc::CMP_NAN,

View file

@ -13,8 +13,8 @@ use syntax::ptr::P;
use utils::{get_trait_def_id, implements_trait, in_external_macro, in_macro, match_path, match_trait_method,
match_type, method_chain_args, return_ty, same_tys, snippet, snippet_opt, span_lint,
span_lint_and_then, span_note_and_lint, walk_ptrs_ty, walk_ptrs_ty_depth};
use utils::{BTREEMAP_ENTRY_PATH, DEFAULT_TRAIT_PATH, HASHMAP_ENTRY_PATH, OPTION_PATH, RESULT_PATH,
VEC_PATH};
use utils::{CSTRING_NEW_PATH, BTREEMAP_ENTRY_PATH, DEFAULT_TRAIT_PATH, HASHMAP_ENTRY_PATH,
OPTION_PATH, RESULT_PATH, VEC_PATH};
use utils::MethodArgs;
#[derive(Clone)]
@ -286,6 +286,33 @@ declare_lint! {
`_.split(\"x\")`"
}
/// **What it does:** This lint checks for getting the inner pointer of a temporary `CString`.
///
/// **Why is this bad?** The inner pointer of a `CString` is only valid as long as the `CString` is
/// alive.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust,ignore
/// let c_str = CString::new("foo").unwrap().as_ptr();
/// unsafe {
/// call_some_ffi_func(c_str);
/// }
/// ```
/// Here `c_str` point to a freed address. The correct use would be:
/// ```rust,ignore
/// let c_str = CString::new("foo").unwrap();
/// unsafe {
/// call_some_ffi_func(c_str.as_ptr());
/// }
/// ```
declare_lint! {
pub TEMPORARY_CSTRING_AS_PTR,
Warn,
"getting the inner pointer of a temporary `CString`"
}
impl LintPass for MethodsPass {
fn get_lints(&self) -> LintArray {
lint_array!(EXTEND_FROM_SLICE,
@ -303,7 +330,8 @@ impl LintPass for MethodsPass {
CLONE_DOUBLE_REF,
NEW_RET_NO_SELF,
SINGLE_CHAR_PATTERN,
SEARCH_IS_SOME)
SEARCH_IS_SOME,
TEMPORARY_CSTRING_AS_PTR)
}
}
@ -334,7 +362,11 @@ impl LateLintPass for MethodsPass {
lint_search_is_some(cx, expr, "rposition", arglists[0], arglists[1]);
} else if let Some(arglists) = method_chain_args(expr, &["extend"]) {
lint_extend(cx, expr, arglists[0]);
} else if let Some(arglists) = method_chain_args(expr, &["unwrap", "as_ptr"]) {
lint_cstring_as_ptr(cx, expr, &arglists[0][0], &arglists[1][0]);
}
lint_or_fun_call(cx, expr, &name.node.as_str(), &args);
if args.len() == 1 && name.node.as_str() == "clone" {
lint_clone_on_copy(cx, expr);
@ -554,6 +586,22 @@ fn lint_extend(cx: &LateContext, expr: &Expr, args: &MethodArgs) {
}
}
fn lint_cstring_as_ptr(cx: &LateContext, expr: &Expr, new: &Expr, unwrap: &Expr) {
if_let_chain!{[
let ExprCall(ref fun, ref args) = new.node,
args.len() == 1,
let ExprPath(None, ref path) = fun.node,
match_path(path, &CSTRING_NEW_PATH),
], {
span_lint_and_then(cx, TEMPORARY_CSTRING_AS_PTR, expr.span,
"you are getting the inner pointer of a temporary `CString`",
|db| {
db.fileline_note(expr.span, "that pointer will be invalid outside this expression");
db.span_help(unwrap.span, "assign the `CString` to a variable to extend its lifetime");
});
}}
}
fn derefs_to_slice(cx: &LateContext, expr: &Expr, ty: &ty::Ty) -> Option<(Span, &'static str)> {
fn may_slice(cx: &LateContext, ty: &ty::Ty) -> bool {
match ty.sty {

View file

@ -28,11 +28,13 @@ pub type MethodArgs = HirVec<P<Expr>>;
// module DefPaths for certain structs/enums we check for
pub const BEGIN_UNWIND: [&'static str; 3] = ["std", "rt", "begin_unwind"];
pub const BOX_NEW_PATH: [&'static str; 4] = ["std", "boxed", "Box", "new"];
pub const BOX_PATH: [&'static str; 3] = ["std", "boxed", "Box"];
pub const BTREEMAP_ENTRY_PATH: [&'static str; 4] = ["collections", "btree", "map", "Entry"];
pub const BTREEMAP_PATH: [&'static str; 4] = ["collections", "btree", "map", "BTreeMap"];
pub const CLONE_PATH: [&'static str; 3] = ["clone", "Clone", "clone"];
pub const CLONE_TRAIT_PATH: [&'static str; 2] = ["clone", "Clone"];
pub const COW_PATH: [&'static str; 3] = ["collections", "borrow", "Cow"];
pub const CSTRING_NEW_PATH: [&'static str; 4] = ["std", "ffi", "CString", "new"];
pub const DEBUG_FMT_METHOD_PATH: [&'static str; 4] = ["std", "fmt", "Debug", "fmt"];
pub const DEFAULT_TRAIT_PATH: [&'static str; 3] = ["core", "default", "Default"];
pub const DISPLAY_FMT_METHOD_PATH: [&'static str; 4] = ["std", "fmt", "Display", "fmt"];
@ -59,7 +61,6 @@ pub const STRING_PATH: [&'static str; 3] = ["collections", "string", "String"];
pub const TRANSMUTE_PATH: [&'static str; 4] = ["core", "intrinsics", "", "transmute"];
pub const VEC_FROM_ELEM_PATH: [&'static str; 3] = ["std", "vec", "from_elem"];
pub const VEC_PATH: [&'static str; 3] = ["collections", "vec", "Vec"];
pub const BOX_PATH: [&'static str; 3] = ["std", "boxed", "Box"];
/// Produce a nested chain of if-lets and ifs from the patterns:
///

View file

@ -470,3 +470,15 @@ fn single_char_pattern() {
//~| HELP try using a char instead:
//~| SUGGESTION x.trim_right_matches('x');
}
#[allow(result_unwrap_used)]
fn temporary_cstring() {
use std::ffi::CString;
( // extra parenthesis to better test spans
//~^ ERROR you are getting the inner pointer of a temporary `CString`
//~| NOTE that pointer will be invalid outside this expression
CString::new("foo").unwrap()
//~^ HELP assign the `CString` to a variable to extend its lifetime
).as_ptr();
}