Clippy cleanup #1244
Clippy cleanup #1244
Conversation
asomers
force-pushed
the
clippy_16_may_2020
branch
from
511c1eb to
441f1ab
Compare
asomers
changed the title
|
@kamalmarhubi @Susurrus @posborne could one of you please review this PR? I especially would like some review for the commits that make previously unsafe functions safe. This PR eliminates almost all of the outstanding clippy issues. It would be great to enable clippy in CI. |
|
I took a quick look, but I think realistically I want to do a closer read, especially since it's been a while since I worked in this codebase. I'll aim to do that in the next few days, by the end of the week at the latest. Does that work? |
|
Yeah; there's no urgency. |
src/sys/ptrace/linux.rs
Outdated
| @@ -173,6 +173,7 @@ libc_bitflags! { | |||
| since="0.10.0", | |||
| note="usages of `ptrace()` should be replaced with the specialized helper functions instead" | |||
| )] | |||
| #[allow(clippy::missing_safety_doc)] // It's deprecated anyway | |||
There was a problem hiding this comment.
Any reason we shouldn't just remove this API at this point? It's been 2.5 years at this point. I'd imagine if we've had at least two releases and it's been at least a year we can probably remove deprecated APIs.
There was a problem hiding this comment.
Good point. I'll take it out.
There was a problem hiding this comment.
Ok, I removed it. But I decided to remove some other stuff, too, and created a new PR: #1255 .
| addr: &sockaddr_storage, | ||
| len: usize) -> Result<SockAddr> { | ||
|
|
||
| assert!(len <= mem::size_of::<sockaddr_un>()); |
There was a problem hiding this comment.
Or debug_assert!? Not certain when you should use which. If we expect this to be on a hot-path might be worth using debug_assert here instead. Otherwise assert is probably fine.
There was a problem hiding this comment.
My usual habit is to use debug_assert for inputs that came from inside of the module, and assert for inputs that cam from outside.
asomers
force-pushed
the
clippy_16_may_2020
branch
2 times, most recently
from
9eae5de to
5079325
Compare
|
bors r+ |
1244: Clippy cleanup r=asomers a=asomers Reported-by: Clippy Co-authored-by: Alan Somers <[email protected]>
|
Build failed: |
|
bors retry |
1244: Clippy cleanup r=asomers a=asomers Reported-by: Clippy Co-authored-by: Alan Somers <[email protected]>
|
Build failed: |
|
redoxer hanged again. I think I'll disable the Redox build if we can't get that issue fixed. bors retry |
1244: Clippy cleanup r=asomers a=asomers Reported-by: Clippy Co-authored-by: Alan Somers <[email protected]>
|
Build failed: |
|
bors retry |
|
Merge conflict. |
Reported-by: Clippy
It already fully validated its arguments, so there's no need for it to be `unsafe`.
It's small and `Copy`, so pass by value is more efficient. This is technically a breaking change, but most code should compile without changes.
All it does is assign a value to a thread-local int. There's nothing unsafe about that.
It was only marked unsafe because it did a pointer cast, but that particular pointer cast is always allowed by C.
asomers
force-pushed
the
clippy_16_may_2020
branch
from
5079325 to
27d39b2
Compare
|
rebased to fix merge conflict and also fix a formatting error in CHANGELOG |
|
bors r+ |
|
Build succeeded: |
Reported-by: Clippy