Skip to content

Commit 402aa63

Browse files
markusmarkus
markus
authored and
markus
committed
Refactoring: simplify use of global variables in backends.
This changes consolidates the three global variables used in the defmt backends into one structure.
1 parent 9fbd524 commit 402aa63

File tree

1 file changed

+41
-23
lines changed
  • firmware/defmt-semihosting/src

1 file changed

+41
-23
lines changed

firmware/defmt-semihosting/src/lib.rs

+41-23
Original file line numberDiff line numberDiff line change
@@ -19,38 +19,56 @@
1919
//! ```
2020
2121
#![no_std]
22-
// nightly warns about static_mut_refs, but 1.76 (our MSRV) does not know about
23-
// that lint yet, so we ignore it it but also ignore if it is unknown
24-
#![allow(unknown_lints, static_mut_refs)]
2522

26-
use core::sync::atomic::{AtomicBool, Ordering};
23+
use core::{
24+
cell::UnsafeCell,
25+
sync::atomic::{AtomicBool, Ordering},
26+
};
2727

2828
use cortex_m_semihosting::hio;
2929

30+
struct ContextInner {
31+
restore_state: critical_section::RestoreState,
32+
encoder: defmt::Encoder,
33+
}
34+
35+
struct Context {
36+
taken: AtomicBool,
37+
inner: UnsafeCell<ContextInner>,
38+
}
39+
40+
// safety: assumes contents are accessed under an isolating critical section.
41+
unsafe impl Sync for Context {}
42+
3043
#[defmt::global_logger]
3144
struct Logger;
3245

33-
static TAKEN: AtomicBool = AtomicBool::new(false);
34-
static mut CS_RESTORE: critical_section::RestoreState = critical_section::RestoreState::invalid();
35-
static mut ENCODER: defmt::Encoder = defmt::Encoder::new();
46+
static CONTEXT: Context = Context {
47+
taken: AtomicBool::new(false),
48+
inner: UnsafeCell::new(ContextInner {
49+
restore_state: critical_section::RestoreState::invalid(),
50+
encoder: defmt::Encoder::new(),
51+
}),
52+
};
3653

3754
unsafe impl defmt::Logger for Logger {
3855
fn acquire() {
3956
// safety: Must be paired with corresponding call to release(), see below
4057
let restore = unsafe { critical_section::acquire() };
4158

42-
if TAKEN.load(Ordering::Relaxed) {
59+
if CONTEXT.taken.load(Ordering::Relaxed) {
4360
panic!("defmt logger taken reentrantly")
4461
}
4562

4663
// no need for CAS because interrupts are disabled
47-
TAKEN.store(true, Ordering::Relaxed);
64+
CONTEXT.taken.store(true, Ordering::Relaxed);
4865

49-
// safety: accessing the `static mut` is OK because we have acquired a critical section.
50-
unsafe { CS_RESTORE = restore };
51-
52-
// safety: accessing the `static mut` is OK because we have disabled interrupts.
53-
unsafe { ENCODER.start_frame(do_write) }
66+
// safety: this assumes the critical section we're in isolates execution
67+
unsafe {
68+
let inner = &mut *CONTEXT.inner.get();
69+
inner.restore_state = restore;
70+
inner.encoder.start_frame(do_write);
71+
}
5472
}
5573

5674
unsafe fn flush() {
@@ -61,21 +79,21 @@ unsafe impl defmt::Logger for Logger {
6179
}
6280

6381
unsafe fn release() {
64-
// safety: accessing the `static mut` is OK because we have disabled interrupts.
65-
ENCODER.end_frame(do_write);
66-
67-
TAKEN.store(false, Ordering::Relaxed);
68-
69-
// safety: accessing the `static mut` is OK because we have acquired a critical section.
70-
let restore = unsafe { CS_RESTORE };
82+
// safety: this assumes the critical section we're in isolates execution
83+
let restore = unsafe {
84+
let inner = &mut *CONTEXT.inner.get();
85+
inner.encoder.end_frame(do_write);
86+
inner.restore_state
87+
};
88+
CONTEXT.taken.store(false, Ordering::Relaxed);
7189

7290
// safety: Must be paired with corresponding call to acquire(), see above
7391
unsafe { critical_section::release(restore) };
7492
}
7593

7694
unsafe fn write(bytes: &[u8]) {
77-
// safety: accessing the `static mut` is OK because we have disabled interrupts.
78-
ENCODER.write(bytes, do_write);
95+
// safety: this assumes the critical section we're in isolates execution
96+
unsafe { (*CONTEXT.inner.get()).encoder.write(bytes, do_write) };
7997
}
8098
}
8199

0 commit comments

Comments
 (0)