Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit 1bcd245

Browse files
committed
Address review comments
1 parent 737bfef commit 1bcd245

File tree

6 files changed

+44
-13
lines changed

6 files changed

+44
-13
lines changed

compiler/rustc_lint/src/methods.rs

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,7 @@ use crate::LateLintPass;
33
use crate::LintContext;
44
use rustc_hir::{Expr, ExprKind, PathSegment};
55
use rustc_middle::ty;
6-
use rustc_span::{
7-
symbol::{sym, Symbol},
8-
ExpnKind, Span,
9-
};
6+
use rustc_span::{symbol::sym, ExpnKind, Span};
107

118
declare_lint! {
129
pub TEMPORARY_CSTRING_AS_PTR,
@@ -59,28 +56,28 @@ impl<'tcx> LateLintPass<'tcx> for TemporaryCStringAsPtr {
5956
}
6057
}
6158

62-
const CSTRING_PATH: [Symbol; 4] = [sym::std, sym::ffi, sym::c_str, sym::CString];
63-
6459
fn lint_cstring_as_ptr(
6560
cx: &LateContext<'_>,
6661
as_ptr_span: Span,
6762
source: &rustc_hir::Expr<'_>,
6863
unwrap: &rustc_hir::Expr<'_>,
6964
) {
7065
let source_type = cx.typeck_results().expr_ty(source);
71-
if let ty::Adt(def, substs) = source_type.kind {
66+
if let ty::Adt(def, substs) = source_type.kind() {
7267
if cx.tcx.is_diagnostic_item(sym::result_type, def.did) {
73-
if let ty::Adt(adt, _) = substs.type_at(0).kind {
74-
if cx.match_def_path(adt.did, &CSTRING_PATH) {
68+
if let ty::Adt(adt, _) = substs.type_at(0).kind() {
69+
if cx.tcx.is_diagnostic_item(sym::cstring_type, adt.did) {
7570
cx.struct_span_lint(TEMPORARY_CSTRING_AS_PTR, as_ptr_span, |diag| {
7671
let mut diag = diag
7772
.build("getting the inner pointer of a temporary `CString`");
7873
diag.span_label(as_ptr_span, "this pointer will be invalid");
7974
diag.span_label(
8075
unwrap.span,
81-
"this `CString` is deallocated at the end of the expression, bind it to a variable to extend its lifetime",
76+
"this `CString` is deallocated at the end of the statement, bind it to a variable to extend its lifetime",
8277
);
83-
diag.note("pointers do not have a lifetime; when calling `as_ptr` the `CString` is deallocated because nothing is referencing it as far as the type system is concerned");
78+
diag.note("pointers do not have a lifetime; when calling `as_ptr` the `CString` will be deallocated at the end of the statement...");
79+
diag.note("...because nothing is referencing it as far as the type system is concerned");
80+
diag.help("for more information, see https://doc.rust-lang.org/reference/destructors.html");
8481
diag.emit();
8582
});
8683
}

compiler/rustc_span/src/symbol.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,7 @@ symbols! {
400400
crate_type,
401401
crate_visibility_modifier,
402402
crt_dash_static: "crt-static",
403+
cstring_type,
403404
ctlz,
404405
ctlz_nonzero,
405406
ctpop,

library/std/src/ffi/c_str.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,9 @@ use crate::sys;
109109
/// documentation of `CString` before use, as improper ownership management
110110
/// of `CString` instances can lead to invalid memory accesses, memory leaks,
111111
/// and other memory errors.
112+
112113
#[derive(PartialEq, PartialOrd, Eq, Ord, Hash, Clone)]
114+
#[cfg_attr(not(test), rustc_diagnostic_item = "cstring_type")]
113115
#[stable(feature = "rust1", since = "1.0.0")]
114116
pub struct CString {
115117
// Invariant 1: the slice ends with a zero byte and has a length of at least one.
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// ignore-tidy-linelength
2+
#![deny(temporary_cstring_as_ptr)]
3+
4+
use std::ffi::CString;
5+
6+
fn some_function(data: *const i8) {}
7+
8+
fn main() {
9+
some_function(CString::new("").unwrap().as_ptr()); //~ ERROR getting the inner pointer of a temporary `CString`
10+
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
error: getting the inner pointer of a temporary `CString`
2+
--> $DIR/lint-temporary-cstring-as-param.rs:9:45
3+
|
4+
LL | some_function(CString::new("").unwrap().as_ptr());
5+
| ------------------------- ^^^^^^ this pointer will be invalid
6+
| |
7+
| this `CString` is deallocated at the end of the statement, bind it to a variable to extend its lifetime
8+
|
9+
note: the lint level is defined here
10+
--> $DIR/lint-temporary-cstring-as-param.rs:2:9
11+
|
12+
LL | #![deny(temporary_cstring_as_ptr)]
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^
14+
= note: pointers do not have a lifetime; when calling `as_ptr` the `CString` will be deallocated at the end of the statement...
15+
= note: ...because nothing is referencing it as far as the type system is concerned
16+
= help: for more information, see https://doc.rust-lang.org/reference/destructors.html
17+
18+
error: aborting due to previous error
19+

src/test/ui/lint/lint-temporary-cstring-as-ptr.stderr

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,16 @@ error: getting the inner pointer of a temporary `CString`
44
LL | let s = CString::new("some text").unwrap().as_ptr();
55
| ---------------------------------- ^^^^^^ this pointer will be invalid
66
| |
7-
| this `CString` is deallocated at the end of the expression, bind it to a variable to extend its lifetime
7+
| this `CString` is deallocated at the end of the statement, bind it to a variable to extend its lifetime
88
|
99
note: the lint level is defined here
1010
--> $DIR/lint-temporary-cstring-as-ptr.rs:2:9
1111
|
1212
LL | #![deny(temporary_cstring_as_ptr)]
1313
| ^^^^^^^^^^^^^^^^^^^^^^^^
14-
= note: pointers do not have a lifetime; when calling `as_ptr` the `CString` is deallocated because nothing is referencing it as far as the type system is concerned
14+
= note: pointers do not have a lifetime; when calling `as_ptr` the `CString` will be deallocated at the end of the statement...
15+
= note: ...because nothing is referencing it as far as the type system is concerned
16+
= help: for more information, see https://doc.rust-lang.org/reference/destructors.html
1517

1618
error: aborting due to previous error
1719

0 commit comments

Comments
 (0)