From c074bec63d14ba0e68a343ebacadb5667f4b42ef Mon Sep 17 00:00:00 2001 From: Stephan Dilly Date: Fri, 23 Jul 2021 13:28:24 +0200 Subject: [PATCH 1/4] safe wrapper for tracing --- Cargo.toml | 1 + src/lib.rs | 2 ++ src/tracing.rs | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 97 insertions(+) create mode 100644 src/tracing.rs diff --git a/Cargo.toml b/Cargo.toml index 1de906fca3..f69bd9097b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,6 +20,7 @@ url = "2.0" bitflags = "1.1.0" libc = "0.2" log = "0.4.8" +lazy_static = "1.4" libgit2-sys = { path = "libgit2-sys", version = "0.12.21" } [target."cfg(all(unix, not(target_os = \"macos\")))".dependencies] diff --git a/src/lib.rs b/src/lib.rs index 52b49fd79f..6972adcb4a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -130,6 +130,7 @@ pub use crate::status::{StatusEntry, StatusIter, StatusOptions, StatusShow, Stat pub use crate::submodule::{Submodule, SubmoduleUpdateOptions}; pub use crate::tag::Tag; pub use crate::time::{IndexTime, Time}; +pub use crate::tracing::{trace_set, TraceLevel}; pub use crate::transaction::Transaction; pub use crate::tree::{Tree, TreeEntry, TreeIter, TreeWalkMode, TreeWalkResult}; pub use crate::treebuilder::TreeBuilder; @@ -690,6 +691,7 @@ mod submodule; mod tag; mod tagforeach; mod time; +mod tracing; mod transaction; mod tree; mod treebuilder; diff --git a/src/tracing.rs b/src/tracing.rs new file mode 100644 index 0000000000..0334146d7b --- /dev/null +++ b/src/tracing.rs @@ -0,0 +1,94 @@ +use lazy_static::lazy_static; +use std::sync::Mutex; + +use libc::c_char; + +use crate::{panic, raw, util::Binding}; + +/// Available tracing levels. When tracing is set to a particular level, +/// callers will be provided tracing at the given level and all lower levels. +#[derive(Copy, Clone, Debug)] +pub enum TraceLevel { + /// No tracing will be performed. + None, + + /// Severe errors that may impact the program's execution + Fatal, + + /// Errors that do not impact the program's execution + Error, + + /// Warnings that suggest abnormal data + Warn, + + /// Informational messages about program execution + Info, + + /// Detailed data that allows for debugging + Debug, + + /// Exceptionally detailed debugging data + Trace, +} + +impl Binding for TraceLevel { + type Raw = raw::git_trace_level_t; + unsafe fn from_raw(raw: raw::git_trace_level_t) -> Self { + match raw { + raw::GIT_TRACE_NONE => Self::None, + raw::GIT_TRACE_FATAL => Self::Fatal, + raw::GIT_TRACE_ERROR => Self::Error, + raw::GIT_TRACE_WARN => Self::Warn, + raw::GIT_TRACE_INFO => Self::Info, + raw::GIT_TRACE_DEBUG => Self::Debug, + raw::GIT_TRACE_TRACE => Self::Trace, + _ => panic!("Unknown git diff binary kind"), + } + } + fn raw(&self) -> raw::git_trace_level_t { + match *self { + Self::None => raw::GIT_TRACE_NONE, + Self::Fatal => raw::GIT_TRACE_FATAL, + Self::Error => raw::GIT_TRACE_ERROR, + Self::Warn => raw::GIT_TRACE_WARN, + Self::Info => raw::GIT_TRACE_INFO, + Self::Debug => raw::GIT_TRACE_DEBUG, + Self::Trace => raw::GIT_TRACE_TRACE, + } + } +} + +pub type TracingCb = Box; + +lazy_static! { + static ref CALLBACK: Mutex> = Mutex::new(None); +} + +/// +pub fn trace_set(level: TraceLevel, cb: T) -> bool +where + T: FnMut(TraceLevel, &str) + Sync + Send + 'static, +{ + if let Ok(mut static_cb) = CALLBACK.lock() { + *static_cb = Some(Box::new(cb)); + + unsafe { + raw::git_trace_set(level.raw(), Some(tracing_cb_c)); + } + + return true; + } + + false +} + +extern "C" fn tracing_cb_c(level: raw::git_trace_level_t, msg: *const c_char) { + panic::wrap(|| unsafe { + if let Ok(mut cb) = CALLBACK.lock() { + if let Some(cb) = cb.as_mut() { + let msg = std::ffi::CStr::from_ptr(msg).to_str().unwrap(); + (*cb)(Binding::from_raw(level), msg); + } + } + }); +} From 9dae63dd3a33960f8b16878df1290552a831c256 Mon Sep 17 00:00:00 2001 From: Stephan Dilly Date: Sat, 24 Jul 2021 02:43:06 +0200 Subject: [PATCH 2/4] try leaky pointer casting approach --- Cargo.toml | 1 - src/tracing.rs | 43 ++++++++++++++++++------------------------- 2 files changed, 18 insertions(+), 26 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f69bd9097b..1de906fca3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,7 +20,6 @@ url = "2.0" bitflags = "1.1.0" libc = "0.2" log = "0.4.8" -lazy_static = "1.4" libgit2-sys = { path = "libgit2-sys", version = "0.12.21" } [target."cfg(all(unix, not(target_os = \"macos\")))".dependencies] diff --git a/src/tracing.rs b/src/tracing.rs index 0334146d7b..1d9a12145e 100644 --- a/src/tracing.rs +++ b/src/tracing.rs @@ -1,5 +1,7 @@ -use lazy_static::lazy_static; -use std::sync::Mutex; +use std::{ + mem::ManuallyDrop, + sync::atomic::{AtomicUsize, Ordering}, +}; use libc::c_char; @@ -42,7 +44,7 @@ impl Binding for TraceLevel { raw::GIT_TRACE_INFO => Self::Info, raw::GIT_TRACE_DEBUG => Self::Debug, raw::GIT_TRACE_TRACE => Self::Trace, - _ => panic!("Unknown git diff binary kind"), + _ => panic!("Unknown git trace level"), } } fn raw(&self) -> raw::git_trace_level_t { @@ -58,37 +60,28 @@ impl Binding for TraceLevel { } } -pub type TracingCb = Box; +pub type TracingCb = fn(TraceLevel, &str); -lazy_static! { - static ref CALLBACK: Mutex> = Mutex::new(None); -} +static CALLBACK: AtomicUsize = AtomicUsize::new(0); /// -pub fn trace_set(level: TraceLevel, cb: T) -> bool -where - T: FnMut(TraceLevel, &str) + Sync + Send + 'static, -{ - if let Ok(mut static_cb) = CALLBACK.lock() { - *static_cb = Some(Box::new(cb)); - - unsafe { - raw::git_trace_set(level.raw(), Some(tracing_cb_c)); - } +pub fn trace_set(level: TraceLevel, cb: TracingCb) -> bool { + let boxed_cb = Box::new(cb); + CALLBACK.store(Box::into_raw(boxed_cb) as *mut _ as usize, Ordering::SeqCst); - return true; + unsafe { + raw::git_trace_set(level.raw(), Some(tracing_cb_c)); } - false + return true; } extern "C" fn tracing_cb_c(level: raw::git_trace_level_t, msg: *const c_char) { panic::wrap(|| unsafe { - if let Ok(mut cb) = CALLBACK.lock() { - if let Some(cb) = cb.as_mut() { - let msg = std::ffi::CStr::from_ptr(msg).to_str().unwrap(); - (*cb)(Binding::from_raw(level), msg); - } - } + let cb = CALLBACK.load(Ordering::SeqCst); + let cb: Box = Box::from_raw(cb as *mut _); + let msg = std::ffi::CStr::from_ptr(msg).to_str().unwrap(); + (*cb)(Binding::from_raw(level), msg); + let _cb = ManuallyDrop::new(cb); }); } From 6ebeaf1890dcdd50cf08292235c1919897d79a28 Mon Sep 17 00:00:00 2001 From: Stephan Dilly Date: Tue, 27 Jul 2021 09:02:46 +0200 Subject: [PATCH 3/4] get rid of box --- src/tracing.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/tracing.rs b/src/tracing.rs index 1d9a12145e..12fe0fce03 100644 --- a/src/tracing.rs +++ b/src/tracing.rs @@ -1,5 +1,4 @@ use std::{ - mem::ManuallyDrop, sync::atomic::{AtomicUsize, Ordering}, }; @@ -66,8 +65,7 @@ static CALLBACK: AtomicUsize = AtomicUsize::new(0); /// pub fn trace_set(level: TraceLevel, cb: TracingCb) -> bool { - let boxed_cb = Box::new(cb); - CALLBACK.store(Box::into_raw(boxed_cb) as *mut _ as usize, Ordering::SeqCst); + CALLBACK.store(cb as usize, Ordering::SeqCst); unsafe { raw::git_trace_set(level.raw(), Some(tracing_cb_c)); @@ -77,11 +75,10 @@ pub fn trace_set(level: TraceLevel, cb: TracingCb) -> bool { } extern "C" fn tracing_cb_c(level: raw::git_trace_level_t, msg: *const c_char) { + let cb = CALLBACK.load(Ordering::SeqCst); panic::wrap(|| unsafe { - let cb = CALLBACK.load(Ordering::SeqCst); - let cb: Box = Box::from_raw(cb as *mut _); + let cb: TracingCb = std::mem::transmute(cb); let msg = std::ffi::CStr::from_ptr(msg).to_str().unwrap(); - (*cb)(Binding::from_raw(level), msg); - let _cb = ManuallyDrop::new(cb); + cb(Binding::from_raw(level), msg); }); } From 29afb68e024f96e2e246641942b7faec48c371bb Mon Sep 17 00:00:00 2001 From: Stephan Dilly Date: Tue, 27 Jul 2021 09:03:24 +0200 Subject: [PATCH 4/4] fmt --- src/tracing.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/tracing.rs b/src/tracing.rs index 12fe0fce03..f348575774 100644 --- a/src/tracing.rs +++ b/src/tracing.rs @@ -1,6 +1,4 @@ -use std::{ - sync::atomic::{AtomicUsize, Ordering}, -}; +use std::sync::atomic::{AtomicUsize, Ordering}; use libc::c_char;