Skip to content

Commit af9685b

Browse files
nico-abramnico-abram
committed
Rename unsafe_sizeof_count_copies to size_of_in_element_count
Also fix review comments: - Use const arrays and iterate them for the method/function names - merge 2 if_chain's into one using a rest pattern - remove unnecessary unsafe block in test And make the lint only point to the count expression instead of the entire function call
1 parent 63a3c44 commit af9685b

File tree

6 files changed

+175
-182
lines changed

6 files changed

+175
-182
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2057,6 +2057,7 @@ Released 2018-09-13
20572057
[`single_element_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_element_loop
20582058
[`single_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match
20592059
[`single_match_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match_else
2060+
[`size_of_in_element_count`]: https://rust-lang.github.io/rust-clippy/master/index.html#size_of_in_element_count
20602061
[`skip_while_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#skip_while_next
20612062
[`slow_vector_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#slow_vector_initialization
20622063
[`stable_sort_primitive`]: https://rust-lang.github.io/rust-clippy/master/index.html#stable_sort_primitive
@@ -2124,7 +2125,6 @@ Released 2018-09-13
21242125
[`unreadable_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#unreadable_literal
21252126
[`unsafe_derive_deserialize`]: https://rust-lang.github.io/rust-clippy/master/index.html#unsafe_derive_deserialize
21262127
[`unsafe_removed_from_name`]: https://rust-lang.github.io/rust-clippy/master/index.html#unsafe_removed_from_name
2127-
[`unsafe_sizeof_count_copies`]: https://rust-lang.github.io/rust-clippy/master/index.html#unsafe_sizeof_count_copies
21282128
[`unsafe_vector_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#unsafe_vector_initialization
21292129
[`unseparated_literal_suffix`]: https://rust-lang.github.io/rust-clippy/master/index.html#unseparated_literal_suffix
21302130
[`unsound_collection_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#unsound_collection_transmute

‎clippy_lints/src/lib.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,7 @@ mod self_assignment;
306306
mod serde_api;
307307
mod shadow;
308308
mod single_component_path_imports;
309+
mod size_of_in_element_count;
309310
mod slow_vector_initialization;
310311
mod stable_sort_primitive;
311312
mod strings;
@@ -329,7 +330,6 @@ mod unnecessary_sort_by;
329330
mod unnecessary_wraps;
330331
mod unnested_or_patterns;
331332
mod unsafe_removed_from_name;
332-
mod unsafe_sizeof_count_copies;
333333
mod unused_io_amount;
334334
mod unused_self;
335335
mod unused_unit;
@@ -917,7 +917,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
917917
&unnecessary_wraps::UNNECESSARY_WRAPS,
918918
&unnested_or_patterns::UNNESTED_OR_PATTERNS,
919919
&unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME,
920-
&unsafe_sizeof_count_copies::UNSAFE_SIZEOF_COUNT_COPIES,
920+
&size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT,
921921
&unused_io_amount::UNUSED_IO_AMOUNT,
922922
&unused_self::UNUSED_SELF,
923923
&unused_unit::UNUSED_UNIT,
@@ -1000,7 +1000,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
10001000
store.register_late_pass(move || box matches::Matches::new(msrv));
10011001
store.register_early_pass(move || box manual_non_exhaustive::ManualNonExhaustive::new(msrv));
10021002
store.register_late_pass(move || box manual_strip::ManualStrip::new(msrv));
1003-
store.register_late_pass(|| box unsafe_sizeof_count_copies::UnsafeSizeofCountCopies);
1003+
store.register_late_pass(|| box size_of_in_element_count::SizeOfInElementCount);
10041004
store.register_late_pass(|| box map_clone::MapClone);
10051005
store.register_late_pass(|| box map_err_ignore::MapErrIgnore);
10061006
store.register_late_pass(|| box shadow::Shadow);
@@ -1608,7 +1608,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
16081608
LintId::of(&unnecessary_sort_by::UNNECESSARY_SORT_BY),
16091609
LintId::of(&unnecessary_wraps::UNNECESSARY_WRAPS),
16101610
LintId::of(&unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME),
1611-
LintId::of(&unsafe_sizeof_count_copies::UNSAFE_SIZEOF_COUNT_COPIES),
1611+
LintId::of(&size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT),
16121612
LintId::of(&unused_io_amount::UNUSED_IO_AMOUNT),
16131613
LintId::of(&unused_unit::UNUSED_UNIT),
16141614
LintId::of(&unwrap::PANICKING_UNWRAP),
@@ -1887,7 +1887,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
18871887
LintId::of(&unit_return_expecting_ord::UNIT_RETURN_EXPECTING_ORD),
18881888
LintId::of(&unnamed_address::FN_ADDRESS_COMPARISONS),
18891889
LintId::of(&unnamed_address::VTABLE_ADDRESS_COMPARISONS),
1890-
LintId::of(&unsafe_sizeof_count_copies::UNSAFE_SIZEOF_COUNT_COPIES),
1890+
LintId::of(&size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT),
18911891
LintId::of(&unused_io_amount::UNUSED_IO_AMOUNT),
18921892
LintId::of(&unwrap::PANICKING_UNWRAP),
18931893
LintId::of(&vec_resize_to_zero::VEC_RESIZE_TO_ZERO),

‎clippy_lints/src/unsafe_sizeof_count_copies.rs‎ renamed to ‎clippy_lints/src/size_of_in_element_count.rs‎

Lines changed: 33 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
//! Lint on unsafe memory copying that use the `size_of` of the pointee type instead of a pointee
2-
//! count
1+
//! Lint on use of `size_of` or `size_of_val` of T in an expression
2+
//! expecting a count of T
33
44
use crate::utils::{match_def_path, paths, span_lint_and_help};
55
use if_chain::if_chain;
@@ -11,15 +11,11 @@ use rustc_session::{declare_lint_pass, declare_tool_lint};
1111

1212
declare_clippy_lint! {
1313
/// **What it does:** Detects expressions where
14-
/// size_of::<T> is used as the count argument to unsafe
15-
/// memory copying functions like ptr::copy and
16-
/// ptr::copy_nonoverlapping where T is the pointee type
17-
/// of the pointers used
14+
/// size_of::<T> or size_of_val::<T> is used as a
15+
/// count of elements of type T
1816
///
1917
/// **Why is this bad?** These functions expect a count
20-
/// of T and not a number of bytes, which can lead to
21-
/// copying the incorrect amount of bytes, which can
22-
/// result in Undefined Behaviour
18+
/// of T and not a number of bytes
2319
///
2420
/// **Known problems:** None.
2521
///
@@ -33,12 +29,12 @@ declare_clippy_lint! {
3329
/// let mut y = [2u8; SIZE];
3430
/// unsafe { copy_nonoverlapping(x.as_ptr(), y.as_mut_ptr(), size_of::<u8>() * SIZE) };
3531
/// ```
36-
pub UNSAFE_SIZEOF_COUNT_COPIES,
32+
pub SIZE_OF_IN_ELEMENT_COUNT,
3733
correctness,
38-
"unsafe memory copying using a byte count instead of a count of T"
34+
"using size_of::<T> or size_of_val::<T> where a count of elements of T is expected"
3935
}
4036

41-
declare_lint_pass!(UnsafeSizeofCountCopies => [UNSAFE_SIZEOF_COUNT_COPIES]);
37+
declare_lint_pass!(SizeOfInElementCount => [SIZE_OF_IN_ELEMENT_COUNT]);
4238

4339
fn get_size_of_ty(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<Ty<'tcx>> {
4440
match expr.kind {
@@ -62,18 +58,30 @@ fn get_size_of_ty(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<Ty<'tc
6258
}
6359
}
6460

61+
const FUNCTIONS: [[&str; 3]; 6] = [
62+
paths::COPY_NONOVERLAPPING,
63+
paths::COPY,
64+
paths::WRITE_BYTES,
65+
paths::PTR_SWAP_NONOVERLAPPING,
66+
paths::PTR_SLICE_FROM_RAW_PARTS,
67+
paths::PTR_SLICE_FROM_RAW_PARTS_MUT,
68+
];
69+
const METHODS: [&str; 5] = [
70+
"write_bytes",
71+
"copy_to",
72+
"copy_from",
73+
"copy_to_nonoverlapping",
74+
"copy_from_nonoverlapping",
75+
];
6576
fn get_pointee_ty_and_count_expr(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<(Ty<'tcx>, &'tcx Expr<'tcx>)> {
6677
if_chain! {
6778
// Find calls to ptr::{copy, copy_nonoverlapping}
6879
// and ptr::{swap_nonoverlapping, write_bytes},
6980
if let ExprKind::Call(func, args) = expr.kind;
70-
if let [_, _, count] = args;
81+
if let [.., count] = args;
7182
if let ExprKind::Path(ref func_qpath) = func.kind;
7283
if let Some(def_id) = cx.qpath_res(func_qpath, func.hir_id).opt_def_id();
73-
if match_def_path(cx, def_id, &paths::COPY_NONOVERLAPPING)
74-
|| match_def_path(cx, def_id, &paths::COPY)
75-
|| match_def_path(cx, def_id, &paths::WRITE_BYTES)
76-
|| match_def_path(cx, def_id, &paths::PTR_SWAP_NONOVERLAPPING);
84+
if FUNCTIONS.iter().any(|func_path| match_def_path(cx, def_id, func_path));
7785

7886
// Get the pointee type
7987
if let Some(pointee_ty) = cx.typeck_results().node_substs(func.hir_id).types().next();
@@ -86,8 +94,7 @@ fn get_pointee_ty_and_count_expr(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -
8694
if let ExprKind::MethodCall(method_path, _, args, _) = expr.kind;
8795
if let [ptr_self, _, count] = args;
8896
let method_ident = method_path.ident.as_str();
89-
if method_ident == "write_bytes" || method_ident == "copy_to" || method_ident == "copy_from"
90-
|| method_ident == "copy_to_nonoverlapping" || method_ident == "copy_from_nonoverlapping";
97+
if METHODS.iter().any(|m| *m == &*method_ident);
9198

9299
// Get the pointee type
93100
if let ty::RawPtr(TypeAndMut { ty: pointee_ty, mutbl:_mutability }) =
@@ -96,31 +103,16 @@ fn get_pointee_ty_and_count_expr(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -
96103
return Some((pointee_ty, count));
97104
}
98105
};
99-
if_chain! {
100-
// Find calls to ptr::copy and copy_nonoverlapping
101-
if let ExprKind::Call(func, args) = expr.kind;
102-
if let [_data, count] = args;
103-
if let ExprKind::Path(ref func_qpath) = func.kind;
104-
if let Some(def_id) = cx.qpath_res(func_qpath, func.hir_id).opt_def_id();
105-
if match_def_path(cx, def_id, &paths::PTR_SLICE_FROM_RAW_PARTS)
106-
|| match_def_path(cx, def_id, &paths::PTR_SLICE_FROM_RAW_PARTS_MUT);
107-
108-
// Get the pointee type
109-
if let Some(pointee_ty) = cx.typeck_results().node_substs(func.hir_id).types().next();
110-
then {
111-
return Some((pointee_ty, count));
112-
}
113-
};
114106
None
115107
}
116108

117-
impl<'tcx> LateLintPass<'tcx> for UnsafeSizeofCountCopies {
109+
impl<'tcx> LateLintPass<'tcx> for SizeOfInElementCount {
118110
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
119-
const HELP_MSG: &str = "use a count of elements instead of a count of bytes \
120-
for the count parameter, it already gets multiplied by the size of the pointed to type";
111+
const HELP_MSG: &str = "use a count of elements instead of a count of bytes\
112+
, it already gets multiplied by the size of the type";
121113

122-
const LINT_MSG: &str = "unsafe memory copying using a byte count \
123-
(multiplied by size_of/size_of_val::<T>) instead of a count of T";
114+
const LINT_MSG: &str = "found a count of bytes \
115+
instead of a count of elements of T";
124116

125117
if_chain! {
126118
// Find calls to unsafe copy functions and get
@@ -134,8 +126,8 @@ impl<'tcx> LateLintPass<'tcx> for UnsafeSizeofCountCopies {
134126
then {
135127
span_lint_and_help(
136128
cx,
137-
UNSAFE_SIZEOF_COUNT_COPIES,
138-
expr.span,
129+
SIZE_OF_IN_ELEMENT_COUNT,
130+
count_expr.span,
139131
LINT_MSG,
140132
None,
141133
HELP_MSG

‎tests/ui/unsafe_sizeof_count_copies.rs‎ renamed to ‎tests/ui/size_of_in_element_count.rs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
#![warn(clippy::unsafe_sizeof_count_copies)]
1+
#![warn(clippy::size_of_in_element_count)]
22

33
use std::mem::{size_of, size_of_val};
44
use std::ptr::{
5-
copy, copy_nonoverlapping, slice_from_raw_parts, slice_from_raw_parts_mut, swap_nonoverlapping, write_bytes,
5+
copy, copy_nonoverlapping, slice_from_raw_parts,
6+
slice_from_raw_parts_mut, swap_nonoverlapping, write_bytes,
67
};
78

89
fn main() {
@@ -29,8 +30,8 @@ fn main() {
2930

3031
unsafe { swap_nonoverlapping(y.as_mut_ptr(), x.as_mut_ptr(), size_of::<u8>() * SIZE) };
3132

32-
unsafe { slice_from_raw_parts_mut(y.as_mut_ptr(), size_of::<u8>() * SIZE) };
33-
unsafe { slice_from_raw_parts(y.as_ptr(), size_of::<u8>() * SIZE) };
33+
slice_from_raw_parts_mut(y.as_mut_ptr(), size_of::<u8>() * SIZE);
34+
slice_from_raw_parts(y.as_ptr(), size_of::<u8>() * SIZE);
3435

3536
// Count expression involving multiplication of size_of (Should trigger the lint)
3637
unsafe { copy_nonoverlapping(x.as_ptr(), y.as_mut_ptr(), size_of::<u8>() * SIZE) };

‎tests/ui/size_of_in_element_count.stderr‎

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
error: found a count of bytes instead of a count of elements of T
2+
--> $DIR/size_of_in_element_count.rs:17:68
3+
|
4+
LL | unsafe { copy_nonoverlapping::<u8>(x.as_ptr(), y.as_mut_ptr(), size_of::<u8>()) };
5+
| ^^^^^^^^^^^^^^^
6+
|
7+
= note: `-D clippy::size-of-in-element-count` implied by `-D warnings`
8+
= help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
9+
10+
error: found a count of bytes instead of a count of elements of T
11+
--> $DIR/size_of_in_element_count.rs:18:62
12+
|
13+
LL | unsafe { copy_nonoverlapping(x.as_ptr(), y.as_mut_ptr(), size_of_val(&x[0])) };
14+
| ^^^^^^^^^^^^^^^^^^
15+
|
16+
= help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
17+
18+
error: found a count of bytes instead of a count of elements of T
19+
--> $DIR/size_of_in_element_count.rs:20:49
20+
|
21+
LL | unsafe { x.as_ptr().copy_to(y.as_mut_ptr(), size_of::<u8>()) };
22+
| ^^^^^^^^^^^^^^^
23+
|
24+
= help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
25+
26+
error: found a count of bytes instead of a count of elements of T
27+
--> $DIR/size_of_in_element_count.rs:21:64
28+
|
29+
LL | unsafe { x.as_ptr().copy_to_nonoverlapping(y.as_mut_ptr(), size_of::<u8>()) };
30+
| ^^^^^^^^^^^^^^^
31+
|
32+
= help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
33+
34+
error: found a count of bytes instead of a count of elements of T
35+
--> $DIR/size_of_in_element_count.rs:22:51
36+
|
37+
LL | unsafe { y.as_mut_ptr().copy_from(x.as_ptr(), size_of::<u8>()) };
38+
| ^^^^^^^^^^^^^^^
39+
|
40+
= help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
41+
42+
error: found a count of bytes instead of a count of elements of T
43+
--> $DIR/size_of_in_element_count.rs:23:66
44+
|
45+
LL | unsafe { y.as_mut_ptr().copy_from_nonoverlapping(x.as_ptr(), size_of::<u8>()) };
46+
| ^^^^^^^^^^^^^^^
47+
|
48+
= help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
49+
50+
error: found a count of bytes instead of a count of elements of T
51+
--> $DIR/size_of_in_element_count.rs:25:47
52+
|
53+
LL | unsafe { copy(x.as_ptr(), y.as_mut_ptr(), size_of::<u8>()) };
54+
| ^^^^^^^^^^^^^^^
55+
|
56+
= help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
57+
58+
error: found a count of bytes instead of a count of elements of T
59+
--> $DIR/size_of_in_element_count.rs:26:47
60+
|
61+
LL | unsafe { copy(x.as_ptr(), y.as_mut_ptr(), size_of_val(&x[0])) };
62+
| ^^^^^^^^^^^^^^^^^^
63+
|
64+
= help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
65+
66+
error: found a count of bytes instead of a count of elements of T
67+
--> $DIR/size_of_in_element_count.rs:28:46
68+
|
69+
LL | unsafe { y.as_mut_ptr().write_bytes(0u8, size_of::<u8>() * SIZE) };
70+
| ^^^^^^^^^^^^^^^^^^^^^^
71+
|
72+
= help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
73+
74+
error: found a count of bytes instead of a count of elements of T
75+
--> $DIR/size_of_in_element_count.rs:29:47
76+
|
77+
LL | unsafe { write_bytes(y.as_mut_ptr(), 0u8, size_of::<u8>() * SIZE) };
78+
| ^^^^^^^^^^^^^^^^^^^^^^
79+
|
80+
= help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
81+
82+
error: found a count of bytes instead of a count of elements of T
83+
--> $DIR/size_of_in_element_count.rs:31:66
84+
|
85+
LL | unsafe { swap_nonoverlapping(y.as_mut_ptr(), x.as_mut_ptr(), size_of::<u8>() * SIZE) };
86+
| ^^^^^^^^^^^^^^^^^^^^^^
87+
|
88+
= help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
89+
90+
error: found a count of bytes instead of a count of elements of T
91+
--> $DIR/size_of_in_element_count.rs:33:46
92+
|
93+
LL | slice_from_raw_parts_mut(y.as_mut_ptr(), size_of::<u8>() * SIZE);
94+
| ^^^^^^^^^^^^^^^^^^^^^^
95+
|
96+
= help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
97+
98+
error: found a count of bytes instead of a count of elements of T
99+
--> $DIR/size_of_in_element_count.rs:34:38
100+
|
101+
LL | slice_from_raw_parts(y.as_ptr(), size_of::<u8>() * SIZE);
102+
| ^^^^^^^^^^^^^^^^^^^^^^
103+
|
104+
= help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
105+
106+
error: found a count of bytes instead of a count of elements of T
107+
--> $DIR/size_of_in_element_count.rs:37:62
108+
|
109+
LL | unsafe { copy_nonoverlapping(x.as_ptr(), y.as_mut_ptr(), size_of::<u8>() * SIZE) };
110+
| ^^^^^^^^^^^^^^^^^^^^^^
111+
|
112+
= help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
113+
114+
error: found a count of bytes instead of a count of elements of T
115+
--> $DIR/size_of_in_element_count.rs:40:62
116+
|
117+
LL | unsafe { copy_nonoverlapping(x.as_ptr(), y.as_mut_ptr(), HALF_SIZE * size_of_val(&x[0]) * 2) };
118+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
119+
|
120+
= help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
121+
122+
error: found a count of bytes instead of a count of elements of T
123+
--> $DIR/size_of_in_element_count.rs:43:47
124+
|
125+
LL | unsafe { copy(x.as_ptr(), y.as_mut_ptr(), DOUBLE_SIZE * size_of::<u8>() / 2) };
126+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
127+
|
128+
= help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
129+
130+
error: aborting due to 16 previous errors
131+

0 commit comments

Comments
 (0)