Skip to content

Fix warnings in str.rs and binding_generated.rs #343

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

Merged
merged 2 commits into from
Jun 4, 2021
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions rust/kernel/str.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,14 @@ impl CStr {
/// must not be mutated.
#[inline]
pub unsafe fn from_char_ptr<'a>(ptr: *const c_types::c_char) -> &'a Self {
// SAFETY: The safety precondition guarantees `ptr` is a valid pointer
// to a `NUL`-terminated C string.
let len = unsafe { bindings::strlen(ptr) } + 1;
unsafe {
Self::from_bytes_with_nul_unchecked(core::slice::from_raw_parts(ptr as _, len as _))
}
// SAFETY: Lifetime guaranteed by the safety precondition.
let bytes = unsafe { core::slice::from_raw_parts(ptr as _, len as _) };
// SAFETY: As `len` is returned by `strlen`, `bytes` does not contain interior `NUL`.
// As we have added 1 to `len`, the last byte is known to be `NUL`.
unsafe { Self::from_bytes_with_nul_unchecked(bytes) }
}

/// Creates a [`CStr`] from a `[u8]`.
Expand Down Expand Up @@ -146,6 +150,7 @@ impl CStr {
// requires `ptr_metadata`).
// While none of them are current stable, it is very likely that one of
// them will eventually be.
// SAFETY: Properties of `bytes` guaranteed by the safety precondition.
unsafe { &*(bytes as *const [u8] as *const Self) }
}

Expand Down Expand Up @@ -188,11 +193,10 @@ impl Index<ops::RangeFrom<usize>> for CStr {
type Output = CStr;

#[inline]
// Clippy false positive
#[allow(clippy::unnecessary_operation)]
fn index(&self, index: ops::RangeFrom<usize>) -> &Self::Output {
// Delegate bounds checking to slice.
&self.as_bytes()[index.start..];
// Assign to _ to mute clippy's unnecessary operation warning.
let _ = &self.as_bytes()[index.start..];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this fix an unsafe warning?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it is for an unused variable warning, yeah, it should be in a separate commit and ideally a separate PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Split into a separate commit. Little reluctant to split into a separate PR since I always feel such a simple change don't warrant its own merge commit ;)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, in the future we will not have merge commits -- i.e. patches will be applied on top of each other in a linear fashion (when I add the sentient module to @ksquirrel :-)

// SAFETY: We just checked the bounds.
unsafe { Self::from_bytes_with_nul_unchecked(&self.0[index.start..]) }
}
Expand Down