Skip to content

Commit 0268a8f

Browse files
jwnrtAlexJones0
authored andcommitted
[sw] Fix SAFETY comments in status.rs
This commit moves the safety comments above what they're annotating. It also adds a safety comment about the `transmute` from bytes loaded from an ELF into an extern C struct. Had to extract `mod_id_ptr` to change the `is_err_status` line's formatting to fit on one line. This is to work around a clippy bug which is fixed in <rust-lang/rust-clippy#11170> but our Rust toolchain is too old. Signed-off-by: James Wainwright <james.wainwright@lowrisc.org>
1 parent 0545a87 commit 0268a8f

File tree

1 file changed

+17
-17
lines changed

1 file changed

+17
-17
lines changed

sw/host/opentitanlib/src/util/status.rs

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,9 @@ pub enum StatusCode {
4747
fn status_create_safe(code: StatusCode, mod_id: u32, file: String, arg: i32) -> RawStatus {
4848
// We do not expect an error since it is a valid String.
4949
let file = CString::new(file).expect("CString::new failed");
50-
unsafe {
51-
// Safety: the function expects a valid readonly C-string which is exactly what
52-
// CString:as_ptr() provides.
53-
status_create(code as u32, mod_id, file.as_ptr(), arg)
54-
}
50+
// SAFETY: the function expects a valid readonly C-string which is exactly what
51+
// CString:as_ptr() provides.
52+
unsafe { status_create(code as u32, mod_id, file.as_ptr(), arg) }
5553
// Note: file is dropped here so the C-string pointer is valid accross the function call.
5654
}
5755

@@ -82,21 +80,20 @@ pub struct Status {
8280
impl Status {
8381
pub fn from_raw_status(status: RawStatus) -> Result<Status> {
8482
// We do not care about the code string but status_extract expects a non-null pointer.
85-
let mut _code_str: *const std::os::raw::c_char = std::ptr::null();
83+
let mut code_str: *const std::os::raw::c_char = std::ptr::null();
8684
let mut arg = 0i32;
8785
let mut mod_id: [std::os::raw::c_char; 3] = [0; 3];
88-
let is_err_status = unsafe {
89-
// Safety: status_extract expects:
90-
// - a non-null pointer to string pointer that will be updated to point
91-
// to the english name of the error code,
92-
// - a non-null pointer to an integer (argument),
93-
// - a non-null pointer to a char[3] buffer that is filled with the module ID.
94-
status_extract(status, &mut _code_str, &mut arg, &mut mod_id as *mut i8)
95-
};
86+
let mod_id_ptr = &mut mod_id as *mut i8;
87+
// SAFETY: status_extract expects:
88+
// - a non-null pointer to string pointer that will be updated to point
89+
// to the english name of the error code,
90+
// - a non-null pointer to an integer (argument),
91+
// - a non-null pointer to a char[3] buffer that is filled with the module ID.
92+
let is_err_status = unsafe { status_extract(status, &mut code_str, &mut arg, mod_id_ptr) };
9693
let code = match is_err_status {
9794
false => StatusCode::Ok,
9895
true => {
99-
// Safety: nothing unsafe except that it's an FFI call.
96+
// SAFETY: nothing unsafe except that it's an FFI call.
10097
let raw_code = unsafe { status_err(status) };
10198
StatusCode::try_from(raw_code)?
10299
}
@@ -239,11 +236,14 @@ pub fn load_elf(elf_file: &PathBuf) -> Result<StatusCreateRecords> {
239236
// it really is safe.
240237
let records = status_create_records
241238
.chunks(RECORD_SIZE)
242-
.map(|chunk| unsafe {
239+
.map(|chunk| {
243240
// We need to provide transmute with a fixed-size array but chunk does not give us one.
244241
// If/When as_chunks is stabilized, we can get rid of this conversion.
245242
let chunk = <[u8; RECORD_SIZE]>::try_from(chunk).unwrap();
246-
std::mem::transmute::<[u8; RECORD_SIZE], ot_status_create_record_t>(chunk)
243+
// SAFETY: `chunk` comes from `struct ot_status_create_record_t` types in C which
244+
// `ot_status_create_record_t` was `bindgen`'d from. Its bytes should always be a
245+
// valid value for this type.
246+
unsafe { std::mem::transmute::<[u8; RECORD_SIZE], ot_status_create_record_t>(chunk) }
247247
})
248248
.map(StatusCreateRecord::try_from)
249249
.collect::<Result<_>>()?;

0 commit comments

Comments
 (0)