Skip to content

Commit

Permalink
log: cleanup and address potential UB
Browse files Browse the repository at this point in the history
This was spawned by the following error message, introduced in 1.77.0:

    warning: creating a shared reference to mutable static is discouraged
      --> src/log/mod.rs:36:27
       |
    36 |     let logger = unsafe { &LOGGER };
       |                           ^^^^^^^ shared reference to mutable static
       |
       = note: for more information, see issue #114447 <rust-lang/rust#114447>

This changes the implementation slightly so that it no longer returns
a reference to the logger itself; instead returning a marker type that
is called in the same way as the original. It also removes the use of
`cortex_m::singleton!`, since the singleton value wasn't actually
being used.

Before this implementation, I tried removing the builder pattern
entirely and just taking the ITM and RTT loggers in the init function.
That ended up being fine, but it prevented any logging (RTT or
otherwise) before the GPIOs were configured.
  • Loading branch information
crawford committed May 12, 2024
1 parent 09fad6d commit 9817637
Showing 1 changed file with 33 additions and 19 deletions.
52 changes: 33 additions & 19 deletions firmware/src/log/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,49 +13,63 @@
// You should have received a copy of the GNU General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.

use core::mem::MaybeUninit;

pub mod itm;
pub mod rtt;

static mut LOGGER: Logger = Logger {
#[cfg(feature = "itm")]
itm: None,
static mut LOGGER: MaybeUninit<Logger> = MaybeUninit::uninit();

#[cfg(feature = "rtt")]
rtt: None,
};
pub fn init() -> InitializedLogger {
static mut INITIALIZED: bool = false;
assert!(unsafe { !INITIALIZED }, "logger already initialized");
unsafe { INITIALIZED = true };

pub fn init() -> &'static Logger {
let logger = unsafe { &LOGGER };
log::set_logger(logger).expect("set_logger");
logger
}
log::set_logger(unsafe {
LOGGER.write(Logger {
#[cfg(feature = "itm")]
itm: None,

pub struct Logger {
#[cfg(feature = "itm")]
itm: Option<itm::Logger>,
#[cfg(feature = "rtt")]
rtt: None,
})
})
.expect("set_logger");

#[cfg(feature = "rtt")]
rtt: Option<rtt::Logger>,
InitializedLogger {}
}

impl Logger {
#[non_exhaustive]
pub struct InitializedLogger {}

impl InitializedLogger {
#[cfg(feature = "itm")]
pub fn add_itm(&self, logger: itm::Logger) -> &Self {
log::set_max_level(log::max_level().max(logger.level));
unsafe { LOGGER.itm = Some(logger) };
unsafe { LOGGER.assume_init_mut().itm = Some(logger) };

log::info!("ITM logging online!");
self
}

#[cfg(feature = "rtt")]
pub fn add_rtt(&self, logger: rtt::Logger) -> &Self {
log::set_max_level(log::max_level().max(logger.level));
unsafe { LOGGER.rtt = Some(logger) };
unsafe { LOGGER.assume_init_mut().rtt = Some(logger) };

log::info!("RTT logging online!");
self
}
}

struct Logger {
#[cfg(feature = "itm")]
itm: Option<itm::Logger>,

#[cfg(feature = "rtt")]
rtt: Option<rtt::Logger>,
}

impl log::Log for Logger {
fn enabled(&self, metadata: &log::Metadata) -> bool {
#[cfg(feature = "itm")]
Expand Down

0 comments on commit 9817637

Please sign in to comment.