-
Notifications
You must be signed in to change notification settings - Fork 457
Clean unsafe
in unsafe fn
s warnings
#348
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -71,12 +71,12 @@ pub trait ModuleParam: core::fmt::Display + core::marker::Sized { | |
let arg = if val.is_null() { | ||
None | ||
} else { | ||
Some(CStr::from_char_ptr(val).as_bytes()) | ||
Some(unsafe { CStr::from_char_ptr(val).as_bytes() }) | ||
}; | ||
match Self::try_from_param_arg(arg) { | ||
Some(new_value) => { | ||
let old_value = (*param).__bindgen_anon_1.arg as *mut Self; | ||
let _ = core::ptr::replace(old_value, new_value); | ||
let old_value = unsafe { (*param).__bindgen_anon_1.arg as *mut Self }; | ||
let _ = unsafe { core::ptr::replace(old_value, new_value) }; | ||
0 | ||
} | ||
None => crate::error::Error::EINVAL.to_kernel_errno(), | ||
|
@@ -95,9 +95,9 @@ pub trait ModuleParam: core::fmt::Display + core::marker::Sized { | |
buf: *mut crate::c_types::c_char, | ||
param: *const crate::bindings::kernel_param, | ||
) -> crate::c_types::c_int { | ||
let slice = core::slice::from_raw_parts_mut(buf as *mut u8, crate::PAGE_SIZE); | ||
let slice = unsafe { core::slice::from_raw_parts_mut(buf as *mut u8, crate::PAGE_SIZE) }; | ||
let mut buf = crate::buffer::Buffer::new(slice); | ||
match write!(buf, "{}\0", *((*param).__bindgen_anon_1.arg as *mut Self)) { | ||
match unsafe { write!(buf, "{}\0", *((*param).__bindgen_anon_1.arg as *mut Self)) } { | ||
Err(_) => crate::error::Error::EINVAL.to_kernel_errno(), | ||
Ok(()) => buf.bytes_written() as crate::c_types::c_int, | ||
} | ||
|
@@ -111,7 +111,7 @@ pub trait ModuleParam: core::fmt::Display + core::marker::Sized { | |
/// | ||
/// The `arg` field of `param` must be an instance of `Self`. | ||
unsafe extern "C" fn free(arg: *mut crate::c_types::c_void) { | ||
core::ptr::drop_in_place(arg as *mut Self); | ||
unsafe { core::ptr::drop_in_place(arg as *mut Self) }; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here and elsewhere you may want to move the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, now that you mention it, I agree. Does anyone know if there a way to tell There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can't find a clippy lint for this. Makes sense to have as a clippy lint though IMO. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 Opened rust-lang/rust-clippy#7322. |
||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The macro could introduce a temp outside of the
unsafe
block that stores the value casted to a raw ptr. Ideally no$foo:expr
happen inside anunsafe
block for macros.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, opening an issue.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this could also be a lint with some logic like:
unsafe fn
...unsafe
block...unsafe_op_in_unsafe_fn
is enabled in this context...unsafe
block.Easier said than done, and I guess I am forgetting a few things, but... :^)
If you think it makes sense, I can open an issue too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe. I believe there has been discussion previously regarding "unsafe" hygiene, but I can't remember what came of it. I think it was on internals.rust-lang.org.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it does not hurt to open it to keep track.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rust-lang/rust-clippy#7323