Skip to content

Commit 6095461

Browse files
molpopgenmolpopgen
authored
Fix several "pedantic" lints from clippy: (#222)
* Fix pedantic lints related to panics * Fix lints due to possible integer wraparound issues * Use safter pointer casts
1 parent ebfde67 commit 6095461

File tree

5 files changed

+60
-25
lines changed

5 files changed

+60
-25
lines changed

‎src/error.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,11 @@ pub fn panic_on_tskit_error(code: i32) {
8181
/// let x = tskit::error::get_tskit_error_message(-207);
8282
/// assert_eq!(x, "Individual out of bounds");
8383
/// ```
84+
///
85+
/// # Panics
86+
///
87+
/// This function must allocate a C string, which may panic
88+
/// if the system runs out of memory.
8489
pub fn get_tskit_error_message(code: i32) -> String {
8590
let c_str = unsafe { std::ffi::CStr::from_ptr(crate::bindings::tsk_strerror(code)) };
8691
c_str.to_str().unwrap().to_owned()

‎src/metadata.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@
164164
//! into `Python` via the `tskit` `Python API`.
165165
166166
use crate::bindings::{tsk_id_t, tsk_size_t};
167-
use crate::SizeType;
167+
use crate::{SizeType, TskitError};
168168
use thiserror::Error;
169169

170170
#[cfg(feature = "derive")]
@@ -230,7 +230,7 @@ impl EncodedMetadata {
230230
if self.encoded.is_empty() {
231231
std::ptr::null()
232232
} else {
233-
self.encoded.as_ptr() as *const libc::c_char
233+
self.encoded.as_ptr().cast::<i8>()
234234
}
235235
}
236236

@@ -277,7 +277,10 @@ pub(crate) fn char_column_to_vector(
277277
}
278278
let mut buffer = vec![];
279279
for i in start..stop {
280-
buffer.push(unsafe { *column.offset(i as isize) } as u8);
280+
match isize::try_from(i) {
281+
Ok(o) => buffer.push(unsafe { *column.offset(o) } as u8),
282+
Err(_) => return Err(TskitError::RangeError("could not convert value to isize")),
283+
};
281284
}
282285
Ok(Some(buffer))
283286
}

‎src/node_table.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ impl<'a> NodeTable<'a> {
119119
pub fn time_array_mut(&mut self) -> &mut [Time] {
120120
unsafe {
121121
std::slice::from_raw_parts_mut(
122-
self.table_.time as *mut Time,
122+
self.table_.time.cast::<Time>(),
123123
self.table_.num_rows as usize,
124124
)
125125
}

‎src/table_collection.rs

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -150,14 +150,17 @@ impl TableCollection {
150150
}
151151

152152
/// Load a table collection from a file.
153+
///
154+
/// # Panics
155+
///
156+
/// This function allocates a `CString` to pass the file name to the C API.
157+
/// A panic will occur if the system runs out of memory.
153158
pub fn new_from_file(filename: &str) -> Result<Self, TskitError> {
154-
let tables = TableCollection::new(1.0); // Arbitrary sequence_length.
155-
match tables {
156-
Ok(_) => (),
159+
// Arbitrary sequence_length.
160+
let mut tables = match TableCollection::new(1.0) {
161+
Ok(t) => (t),
157162
Err(e) => return Err(e),
158-
}
159-
160-
let mut tables = tables.unwrap();
163+
};
161164

162165
let c_str = std::ffi::CString::new(filename).unwrap();
163166
let rv = unsafe {
@@ -241,9 +244,9 @@ impl TableCollection {
241244
ll_bindings::tsk_individual_table_add_row(
242245
&mut (*self.as_mut_ptr()).individuals,
243246
flags,
244-
location.as_ptr() as *const f64,
247+
location.as_ptr().cast::<f64>(),
245248
location.len() as tsk_size_t,
246-
parents.as_ptr() as *const tsk_id_t,
249+
parents.as_ptr().cast::<tsk_id_t>(),
247250
parents.len() as tsk_size_t,
248251
std::ptr::null(),
249252
0,
@@ -269,9 +272,9 @@ impl TableCollection {
269272
ll_bindings::tsk_individual_table_add_row(
270273
&mut (*self.as_mut_ptr()).individuals,
271274
flags,
272-
location.as_ptr() as *const f64,
275+
location.as_ptr().cast::<f64>(),
273276
location.len() as tsk_size_t,
274-
parents.as_ptr() as *const tsk_id_t,
277+
parents.as_ptr().cast::<tsk_id_t>(),
275278
parents.len() as tsk_size_t,
276279
md.as_ptr(),
277280
md.len().into(),
@@ -687,6 +690,10 @@ impl TableCollection {
687690

688691
/// Dump the table collection to file.
689692
///
693+
/// # Panics
694+
///
695+
/// This function allocates a `CString` to pass the file name to the C API.
696+
/// A panic will occur if the system runs out of memory.
690697
pub fn dump(&self, filename: &str, options: TableOutputOptions) -> TskReturnValue {
691698
let c_str = std::ffi::CString::new(filename).unwrap();
692699
let rv = unsafe {
@@ -770,11 +777,11 @@ impl TableCollection {
770777
let rv = unsafe {
771778
ll_bindings::tsk_table_collection_simplify(
772779
self.as_mut_ptr(),
773-
samples.as_ptr() as *const tsk_id_t,
780+
samples.as_ptr().cast::<tsk_id_t>(),
774781
samples.len() as tsk_size_t,
775782
options.bits(),
776783
match idmap {
777-
true => output_node_map.as_mut_ptr() as *mut tsk_id_t,
784+
true => output_node_map.as_mut_ptr().cast::<tsk_id_t>(),
778785
false => std::ptr::null_mut(),
779786
},
780787
)

‎src/trees.rs

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,12 @@ impl Tree {
4848
fn wrap(num_nodes: tsk_size_t, flags: TreeFlags) -> Self {
4949
let temp = unsafe {
5050
libc::malloc(std::mem::size_of::<ll_bindings::tsk_tree_t>())
51-
as *mut ll_bindings::tsk_tree_t
51+
.cast::<ll_bindings::tsk_tree_t>()
5252
};
5353
if temp.is_null() {
5454
panic!("out of memory");
5555
}
56-
let mbox = unsafe { MBox::from_raw(temp as *mut ll_bindings::tsk_tree_t) };
56+
let mbox = unsafe { MBox::from_raw(temp.cast::<ll_bindings::tsk_tree_t>()) };
5757
Self {
5858
inner: mbox,
5959
current_tree: 0,
@@ -410,14 +410,21 @@ impl Tree {
410410

411411
/// Obtain the list of samples for the current tree/tree sequence
412412
/// as a vector.
413+
///
414+
/// # Panics
415+
///
416+
/// Will panic if the number of samples is too large to cast to a valid id.
413417
#[deprecated(since = "0.2.3", note = "Please use Tree::sample_nodes instead")]
414418
pub fn samples_to_vec(&self) -> Vec<NodeId> {
415419
let num_samples =
416420
unsafe { ll_bindings::tsk_treeseq_get_num_samples((*self.inner).tree_sequence) };
417421
let mut rv = vec![];
418422

419423
for i in 0..num_samples {
420-
let u = unsafe { *(*(*self.inner).tree_sequence).samples.offset(i as isize) };
424+
let u = match isize::try_from(i) {
425+
Ok(o) => unsafe { *(*(*self.inner).tree_sequence).samples.offset(o) },
426+
Err(e) => panic!("{e}"),
427+
};
421428
rv.push(u.into());
422429
}
423430
rv
@@ -713,8 +720,8 @@ impl<'a> PostorderNodeIterator<'a> {
713720
ll_bindings::tsk_tree_postorder(
714721
tree.as_ptr(),
715722
NodeId::NULL.into(), // start from virtual root
716-
nodes.as_mut_ptr() as *mut tsk_id_t,
717-
ptr as *mut tsk_size_t,
723+
nodes.as_mut_ptr().cast::<tsk_id_t>(),
724+
ptr.cast::<tsk_size_t>(),
718725
)
719726
};
720727

@@ -999,6 +1006,10 @@ impl TreeSequence {
9991006
/// This behavior may change in a future release, which could
10001007
/// break `API`.
10011008
///
1009+
/// # Panics
1010+
///
1011+
/// This function allocates a `CString` to pass the file name to the C API.
1012+
/// A panic will occur if the system runs out of memory.
10021013
pub fn dump(&self, filename: &str, options: TableOutputOptions) -> TskReturnValue {
10031014
let c_str = std::ffi::CString::new(filename).unwrap();
10041015
let rv =
@@ -1077,6 +1088,9 @@ impl TreeSequence {
10771088
}
10781089

10791090
/// Get the list of samples as a vector.
1091+
/// # Panics
1092+
///
1093+
/// Will panic if the number of samples is too large to cast to a valid id.
10801094
#[deprecated(
10811095
since = "0.2.3",
10821096
note = "Please use TreeSequence::sample_nodes instead"
@@ -1086,7 +1100,10 @@ impl TreeSequence {
10861100
let mut rv = vec![];
10871101

10881102
for i in 0..num_samples {
1089-
let u = NodeId::from(unsafe { *(*self.as_ptr()).samples.offset(i as isize) });
1103+
let u = match isize::try_from(i) {
1104+
Ok(o) => NodeId::from(unsafe { *(*self.as_ptr()).samples.offset(o) }),
1105+
Err(e) => panic!("{e}"),
1106+
};
10901107
rv.push(u);
10911108
}
10921109
rv
@@ -1148,7 +1165,10 @@ impl TreeSequence {
11481165
idmap: bool,
11491166
) -> Result<(Self, Option<Vec<NodeId>>), TskitError> {
11501167
let mut tables = TableCollection::new(unsafe { (*(*self.inner).tables).sequence_length })?;
1151-
tables.build_index().unwrap();
1168+
match tables.build_index() {
1169+
Ok(_) => (),
1170+
Err(e) => return Err(e),
1171+
}
11521172
let mut ts = tables.tree_sequence(TreeSequenceFlags::default())?;
11531173
let mut output_node_map: Vec<NodeId> = vec![];
11541174
if idmap {
@@ -1158,12 +1178,12 @@ impl TreeSequence {
11581178
ll_bindings::tsk_treeseq_simplify(
11591179
self.as_ptr(),
11601180
// NOTE: casting away const-ness:
1161-
samples.as_ptr() as *mut tsk_id_t,
1181+
samples.as_ptr().cast::<tsk_id_t>(),
11621182
samples.len() as tsk_size_t,
11631183
options.bits(),
11641184
ts.as_mut_ptr(),
11651185
match idmap {
1166-
true => output_node_map.as_mut_ptr() as *mut tsk_id_t,
1186+
true => output_node_map.as_mut_ptr().cast::<tsk_id_t>(),
11671187
false => std::ptr::null_mut(),
11681188
},
11691189
)

0 commit comments

Comments
 (0)