From 8934f31ac616123315a92386fbc8fd32ac1afdb7 Mon Sep 17 00:00:00 2001 From: Craig Colegrove Date: Thu, 8 Jul 2021 13:40:54 -0700 Subject: [PATCH 1/5] Add new feature flag for leaking string memory --- Cargo.toml | 3 ++- lib.rs | 30 ++++++++++++++++++++++++------ 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 4070f0c..7d8b80d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "slog-stdlog" -version = "4.1.0" +version = "4.2.0" authors = ["Dawid Ciężarkiewicz "] description = "`log` crate adapter for slog-rs" keywords = ["slog", "logging", "json", "log"] @@ -17,6 +17,7 @@ path = "lib.rs" [features] default = [] kv_unstable = ["log/kv_unstable_std", "slog/dynamic-keys"] +record_location_unstable = [] [dependencies] slog = "2.4" diff --git a/lib.rs b/lib.rs index 842b7ce..f0afab8 100644 --- a/lib.rs +++ b/lib.rs @@ -53,8 +53,8 @@ extern crate log; #[cfg(feature = "kv_unstable")] mod kv; +use slog::{b, Level, KV}; use std::{fmt, io}; -use slog::{Level, KV, b}; struct Logger; @@ -69,7 +69,21 @@ fn log_to_slog_level(level: log::Level) -> Level { } fn record_as_location(r: &log::Record) -> slog::RecordLocation { + // Warning: expands Record module and file names for non-static strings by leaking strings + #[cfg(feature = "record_location_unstable")] + let module = r.module_path_static().unwrap_or(match r.module_path() { + Some(s) => Box::leak(s.to_string().into_boxed_str()), + None => "unknown", + }); + #[cfg(feature = "record_location_unstable")] + let file = r.file_static().unwrap_or(match r.file() { + Some(s) => Box::leak(s.to_string().into_boxed_str()), + None => "unknown", + }); + + #[cfg(not(feature = "record_location_unstable"))] let module = r.module_path_static().unwrap_or(""); + #[cfg(not(feature = "record_location_unstable"))] let file = r.file_static().unwrap_or(""); let line = r.line().unwrap_or_default(); @@ -100,10 +114,10 @@ impl log::Log for Logger { }; #[cfg(feature = "kv_unstable")] { - let key_values = r.key_values(); - let mut visitor = kv::Visitor::new(); - key_values.visit(&mut visitor).unwrap(); - slog_scope::with_logger(|logger| logger.log(&slog::Record::new(&s, args, b!(visitor)))) + let key_values = r.key_values(); + let mut visitor = kv::Visitor::new(); + key_values.visit(&mut visitor).unwrap(); + slog_scope::with_logger(|logger| logger.log(&slog::Record::new(&s, args, b!(visitor)))) } #[cfg(not(feature = "kv_unstable"))] slog_scope::with_logger(|logger| logger.log(&slog::Record::new(&s, args, b!()))) @@ -252,7 +266,11 @@ impl slog::Drain for StdLog { let lazy = LazyLogString::new(info, logger_values); // Please don't yell at me for this! :D // https://github.com/rust-lang-nursery/log/issues/95 - log::__private_api_log(format_args!("{}", lazy), level, &(target, info.module(), info.file(), info.line())); + log::__private_api_log( + format_args!("{}", lazy), + level, + &(target, info.module(), info.file(), info.line()), + ); Ok(()) } From 3335ffd8f02ebf379fc0bb2e0b597c766d2420fd Mon Sep 17 00:00:00 2001 From: Craig Colegrove Date: Thu, 8 Jul 2021 13:44:32 -0700 Subject: [PATCH 2/5] Remove some auto-formatter changes --- lib.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/lib.rs b/lib.rs index f0afab8..064cc08 100644 --- a/lib.rs +++ b/lib.rs @@ -114,10 +114,10 @@ impl log::Log for Logger { }; #[cfg(feature = "kv_unstable")] { - let key_values = r.key_values(); - let mut visitor = kv::Visitor::new(); - key_values.visit(&mut visitor).unwrap(); - slog_scope::with_logger(|logger| logger.log(&slog::Record::new(&s, args, b!(visitor)))) + let key_values = r.key_values(); + let mut visitor = kv::Visitor::new(); + key_values.visit(&mut visitor).unwrap(); + slog_scope::with_logger(|logger| logger.log(&slog::Record::new(&s, args, b!(visitor)))) } #[cfg(not(feature = "kv_unstable"))] slog_scope::with_logger(|logger| logger.log(&slog::Record::new(&s, args, b!()))) @@ -266,11 +266,7 @@ impl slog::Drain for StdLog { let lazy = LazyLogString::new(info, logger_values); // Please don't yell at me for this! :D // https://github.com/rust-lang-nursery/log/issues/95 - log::__private_api_log( - format_args!("{}", lazy), - level, - &(target, info.module(), info.file(), info.line()), - ); + log::__private_api_log(format_args!("{}", lazy), level, &(target, info.module(), info.file(), info.line())); Ok(()) } From 95ad3ea1474d2cb5824e5b91664c6150ff73f12b Mon Sep 17 00:00:00 2001 From: Craig Colegrove Date: Mon, 12 Jul 2021 10:26:10 -0700 Subject: [PATCH 3/5] Only leak each string once, update CHANGELOG.md --- CHANGELOG.md | 13 +++++++++++++ Cargo.toml | 1 + lib.rs | 48 ++++++++++++++++++++++++++++++++---------------- 3 files changed, 46 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f4563a4..e9c3168 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,19 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +## 4.2.0 - 2021-7-12 +### Changed + +* Add feature flag `record_location_unstable` to expand record location support. + Be warned that this is accomplished by leaking the record location strings. + +## 4.1.0 - 2020-10-21 +### Changed + +* Require `slog` 2.4 or greater +* Remove dependency `crossbeam` +* Require `log` 0.4.11 or greater + ## 4.0.0 - 2018-08-13 ### Changed diff --git a/Cargo.toml b/Cargo.toml index 7d8b80d..2e10104 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,6 +23,7 @@ record_location_unstable = [] slog = "2.4" slog-scope = "4" log = { version = "0.4.11", features = ["std"] } +cached = "0.23.0" [dev-dependencies] slog-term = "2" diff --git a/lib.rs b/lib.rs index 064cc08..14e50b1 100644 --- a/lib.rs +++ b/lib.rs @@ -68,23 +68,35 @@ fn log_to_slog_level(level: log::Level) -> Level { } } -fn record_as_location(r: &log::Record) -> slog::RecordLocation { - // Warning: expands Record module and file names for non-static strings by leaking strings - #[cfg(feature = "record_location_unstable")] - let module = r.module_path_static().unwrap_or(match r.module_path() { - Some(s) => Box::leak(s.to_string().into_boxed_str()), +/// Gets a static string for Record location information. +/// Uses a cache to avoid leaking each string more than once. +#[cfg(feature = "record_location_unstable")] +#[cached::proc_macro::cached] +fn get_static_info( + static_info: Option<&'static str>, + non_static_info: Option, +) -> &'static str { + static_info.unwrap_or(match non_static_info { + Some(s) => Box::leak(s.into_boxed_str()), None => "unknown", - }); - #[cfg(feature = "record_location_unstable")] - let file = r.file_static().unwrap_or(match r.file() { - Some(s) => Box::leak(s.to_string().into_boxed_str()), - None => "unknown", - }); + }) +} +fn record_as_location(r: &log::Record) -> slog::RecordLocation { #[cfg(not(feature = "record_location_unstable"))] let module = r.module_path_static().unwrap_or(""); #[cfg(not(feature = "record_location_unstable"))] let file = r.file_static().unwrap_or(""); + + // Warning: expands Record module and file names for non-static strings by leaking strings + #[cfg(feature = "record_location_unstable")] + let module = get_static_info( + r.module_path_static(), + r.module_path().map(|s| s.to_string()), + ); + #[cfg(feature = "record_location_unstable")] + let file = get_static_info(r.file_static(), r.file().map(|s| s.to_string())); + let line = r.line().unwrap_or_default(); slog::RecordLocation { @@ -114,10 +126,10 @@ impl log::Log for Logger { }; #[cfg(feature = "kv_unstable")] { - let key_values = r.key_values(); - let mut visitor = kv::Visitor::new(); - key_values.visit(&mut visitor).unwrap(); - slog_scope::with_logger(|logger| logger.log(&slog::Record::new(&s, args, b!(visitor)))) + let key_values = r.key_values(); + let mut visitor = kv::Visitor::new(); + key_values.visit(&mut visitor).unwrap(); + slog_scope::with_logger(|logger| logger.log(&slog::Record::new(&s, args, b!(visitor)))) } #[cfg(not(feature = "kv_unstable"))] slog_scope::with_logger(|logger| logger.log(&slog::Record::new(&s, args, b!()))) @@ -266,7 +278,11 @@ impl slog::Drain for StdLog { let lazy = LazyLogString::new(info, logger_values); // Please don't yell at me for this! :D // https://github.com/rust-lang-nursery/log/issues/95 - log::__private_api_log(format_args!("{}", lazy), level, &(target, info.module(), info.file(), info.line())); + log::__private_api_log( + format_args!("{}", lazy), + level, + &(target, info.module(), info.file(), info.line()), + ); Ok(()) } From d6305fefebfe8b26011f8ba9f065de38410b2f35 Mon Sep 17 00:00:00 2001 From: Craig Colegrove Date: Mon, 12 Jul 2021 10:44:34 -0700 Subject: [PATCH 4/5] Make cached an optional dependency --- Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 2e10104..0a84aef 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,13 +17,13 @@ path = "lib.rs" [features] default = [] kv_unstable = ["log/kv_unstable_std", "slog/dynamic-keys"] -record_location_unstable = [] +record_location_unstable = ["cached"] [dependencies] slog = "2.4" slog-scope = "4" log = { version = "0.4.11", features = ["std"] } -cached = "0.23.0" +cached = { version = "0.23.0", optional = true } [dev-dependencies] slog-term = "2" From 3f278ec4104629545c597074fea7394698cc8671 Mon Sep 17 00:00:00 2001 From: Craig Colegrove Date: Mon, 12 Jul 2021 15:30:14 -0700 Subject: [PATCH 5/5] Don't unnecessarily clone already-static strings --- lib.rs | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/lib.rs b/lib.rs index 14e50b1..b0d10fe 100644 --- a/lib.rs +++ b/lib.rs @@ -72,14 +72,11 @@ fn log_to_slog_level(level: log::Level) -> Level { /// Uses a cache to avoid leaking each string more than once. #[cfg(feature = "record_location_unstable")] #[cached::proc_macro::cached] -fn get_static_info( - static_info: Option<&'static str>, - non_static_info: Option, -) -> &'static str { - static_info.unwrap_or(match non_static_info { +fn get_static_info(non_static_info: Option) -> &'static str { + match non_static_info { Some(s) => Box::leak(s.into_boxed_str()), None => "unknown", - }) + } } fn record_as_location(r: &log::Record) -> slog::RecordLocation { @@ -90,12 +87,13 @@ fn record_as_location(r: &log::Record) -> slog::RecordLocation { // Warning: expands Record module and file names for non-static strings by leaking strings #[cfg(feature = "record_location_unstable")] - let module = get_static_info( - r.module_path_static(), - r.module_path().map(|s| s.to_string()), - ); + let module = r + .module_path_static() + .unwrap_or_else(|| get_static_info(r.module_path().map(|s| s.to_string()))); #[cfg(feature = "record_location_unstable")] - let file = get_static_info(r.file_static(), r.file().map(|s| s.to_string())); + let file = r + .file_static() + .unwrap_or_else(|| get_static_info(r.file().map(|s| s.to_string()))); let line = r.line().unwrap_or_default();