Skip to content

Commit 6a36313

Browse files
committed
Fix UB if calling is_done more than once on Transfer, add comment for use of Option in inner
1 parent 31ce4ef commit 6a36313

File tree

1 file changed

+4
-2
lines changed

1 file changed

+4
-2
lines changed

nrf-hal-common/src/spis.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,7 @@ where
420420

421421
/// A DMA transfer
422422
pub struct Transfer<T: Instance, B> {
423+
// FIXME: Always `Some`, only using `Option` here to allow moving fields out of `inner`.
423424
inner: Option<Inner<T, B>>,
424425
}
425426

@@ -463,6 +464,7 @@ impl<T: Instance, B> Drop for Transfer<T, B> {
463464
}
464465
/// A full duplex DMA transfer
465466
pub struct TransferSplit<T: Instance, TxB, RxB> {
467+
// FIXME: Always `Some`, only using `Option` here to allow moving fields out of `inner`.
466468
inner: Option<InnerSplit<T, TxB, RxB>>,
467469
}
468470

@@ -490,15 +492,15 @@ impl<T: Instance, TxB, RxB> TransferSplit<T, TxB, RxB> {
490492
pub fn is_done(&mut self) -> bool {
491493
let inner = self
492494
.inner
493-
.take()
495+
.as_mut()
494496
.unwrap_or_else(|| unsafe { core::hint::unreachable_unchecked() });
495497
inner.spis.is_done()
496498
}
497499
}
498500

499501
impl<T: Instance, TxB, RxB> Drop for TransferSplit<T, TxB, RxB> {
500502
fn drop(&mut self) {
501-
if let Some(inner) = self.inner.as_mut() {
503+
if let Some(inner) = self.inner.take() {
502504
compiler_fence(Ordering::SeqCst);
503505
while !inner.spis.is_done() {}
504506
inner.spis.disable();

0 commit comments

Comments
 (0)