From 0b0a78ccf2af22a51e737cd5c3e34c2aadc7af55 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 9 Apr 2020 21:24:21 -0400 Subject: [PATCH 1/2] Reduce the scope of `unsafe` blocks --- src/lib.rs | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 944e4d7ec..02e129673 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1257,19 +1257,21 @@ fn set_logger_inner(make_logger: F) -> Result<(), SetLoggerError> where F: FnOnce() -> &'static dyn Log, { - unsafe { - match STATE.compare_and_swap(UNINITIALIZED, INITIALIZING, Ordering::SeqCst) { - UNINITIALIZED => { + match STATE.compare_and_swap(UNINITIALIZED, INITIALIZING, Ordering::SeqCst) { + UNINITIALIZED => { + unsafe { LOGGER = make_logger(); - STATE.store(INITIALIZED, Ordering::SeqCst); - Ok(()) } - INITIALIZING => { - while STATE.load(Ordering::SeqCst) == INITIALIZING {} - Err(SetLoggerError(())) + STATE.store(INITIALIZED, Ordering::SeqCst); + Ok(()) + } + INITIALIZING => { + while STATE.load(Ordering::SeqCst) == INITIALIZING { + std::sync::atomic::spin_loop_hint(); } - _ => Err(SetLoggerError(())), + Err(SetLoggerError(())) } + _ => Err(SetLoggerError(())), } } @@ -1345,13 +1347,11 @@ impl error::Error for ParseLevelError {} /// /// If a logger has not been set, a no-op implementation is returned. pub fn logger() -> &'static dyn Log { - unsafe { - if STATE.load(Ordering::SeqCst) != INITIALIZED { - static NOP: NopLogger = NopLogger; - &NOP - } else { - LOGGER - } + if STATE.load(Ordering::SeqCst) != INITIALIZED { + static NOP: NopLogger = NopLogger; + &NOP + } else { + unsafe { LOGGER } } } From 51562abb06de57e5dcd8ef5254e9930173544edb Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 10 Apr 2020 13:23:03 -0400 Subject: [PATCH 2/2] Document why `transmute` is sound --- src/lib.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 02e129673..fceda8044 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -331,7 +331,10 @@ pub enum Level { /// The "error" level. /// /// Designates very serious errors. - Error = 1, // This way these line up with the discriminants for LevelFilter below + // This way these line up with the discriminants for LevelFilter below + // This works because Rust treats field-less enums the same way as C does: + // https://doc.rust-lang.org/reference/items/enumerations.html#custom-discriminant-values-for-field-less-enumerations + Error = 1, /// The "warn" level. /// /// Designates hazardous situations. @@ -1173,6 +1176,12 @@ pub fn set_max_level(level: LevelFilter) { /// [`set_max_level`]: fn.set_max_level.html #[inline(always)] pub fn max_level() -> LevelFilter { + // Since `LevelFilter` is `repr(usize)`, + // this transmute is sound if and only if `MAX_LOG_LEVEL_FILTER` + // is set to a usize that is a valid discriminant for `LevelFilter`. + // Since `MAX_LOG_LEVEL_FILTER` is private, the only time it's set + // is by `set_max_level` above, i.e. by casting a `LevelFilter` to `usize`. + // So any usize stored in `MAX_LOG_LEVEL_FILTER` is a valid discriminant. unsafe { mem::transmute(MAX_LOG_LEVEL_FILTER.load(Ordering::Relaxed)) } }