Mark set_print as unsafe

examples/capable/src/main.rs

@@ -175,7 +175,12 @@ fn main() -> Result<()> {
 
     let mut skel_builder = CapableSkelBuilder::default();
     if opts.debug {
-        skel_builder.obj_builder.debug(true);
+        unsafe {
+            // SAFETY:
+            // no other thread is running which could cause undefined behaviour due
+            // to this call.
+            skel_builder.obj_builder.debug(true);
+        }
     }
 
     bump_memlock_rlimit()?;

examples/runqslower/src/main.rs

@@ -79,7 +79,12 @@ fn main() -> Result<()> {
 
     let mut skel_builder = RunqslowerSkelBuilder::default();
     if opts.verbose {
-        skel_builder.obj_builder.debug(true);
+        unsafe {
+            // SAFETY:
+            // no other thread is running which could cause undefined behaviour due
+            // to this call.
+            skel_builder.obj_builder.debug(true);
+        }
     }
 
     bump_memlock_rlimit()?;

examples/tproxy/src/main.rs

@@ -53,7 +53,12 @@ fn main() -> Result<()> {
 
     let mut skel_builder = TproxySkelBuilder::default();
     if opts.verbose {
-        skel_builder.obj_builder.debug(true);
+        unsafe {
+            // SAFETY:
+            // no other thread is running which could cause undefined behaviour due
+            // to this call.
+            skel_builder.obj_builder.debug(true);
+        }
     }
 
     // Set constants

libbpf-rs/src/object.rs

@@ -32,14 +32,17 @@ impl ObjectBuilder {
 
     /// Option to print debug output to stderr.
     ///
+    /// # Safety
+    ///
+    /// See [`libbpf-rs::set_print`].
+    /// Also it is guarenteed that the only part of the `ObjectBuilder` that interacts with libbpf
+    /// are the [`open_file`] and [`open_memory`] methods.
+    ///
     /// Note: This function uses [`set_print`] internally and will overwrite any callbacks
     /// currently in use.
-    pub fn debug(&mut self, dbg: bool) -> &mut Self {
-        if dbg {
-            set_print(Some((PrintLevel::Debug, |_, s| print!("{s}"))));
-        } else {
-            set_print(None);
-        }
+    pub unsafe fn debug(&mut self, dbg: bool) -> &mut Self {
+        let callback = dbg.then(|| (PrintLevel::Debug, (|_, s| print!("{s}")) as PrintCallback));
+        unsafe { set_print(callback) };
         self
     }
 

libbpf-rs/src/print.rs

@@ -69,6 +69,9 @@ extern "C" fn outer_print_cb(
     0 // return value is ignored by libbpf
 }
 
+// allow main to reinforce the idea that set print should be used before any other interaction
+// with the library.
+#[allow(clippy::needless_doctest_main)]
 /// Set a callback to receive log messages from libbpf, instead of printing them to stderr.
 ///
 /// # Arguments
@@ -78,6 +81,12 @@ extern "C" fn outer_print_cb(
 ///
 /// This overrides (and is overridden by) [`ObjectBuilder::debug`]
 ///
+/// # Safety
+///
+/// This function mutates a global variable without syncronization inside libbpf. This means that
+/// it is not thread safe and **cannot** be called concurrently with any other function
+/// in this library.
+///
 /// # Examples
 ///
 /// To pass all messages to the `log` crate:
@@ -93,48 +102,35 @@ extern "C" fn outer_print_cb(
 ///     }
 /// }
 ///
-/// set_print(Some((PrintLevel::Debug, print_to_log)));
+/// fn main() {
+///     unsafe {
+///         // SAFETY: this is being setup before interacting with any other part of the library
+///         set_print(Some((PrintLevel::Debug, print_to_log)))
+///     };
+/// }
 /// ```
 ///
 /// To disable printing completely:
 ///
 /// ```
 /// use libbpf_rs::set_print;
-/// set_print(None);
-/// ```
-///
-/// To temporarliy suppress output:
-///
-/// ```
-/// use libbpf_rs::set_print;
-///
-/// let prev = set_print(None);
-/// // do things quietly
-/// set_print(prev);
+/// fn main() {
+///     unsafe {
+///         // SAFETY: this is being setup before interacting with any other part of the library
+///         set_print(None)
+///     };
+/// }
 /// ```
-pub fn set_print(
+pub unsafe fn set_print(
     mut callback: Option<(PrintLevel, PrintCallback)>,
 ) -> Option<(PrintLevel, PrintCallback)> {
-    let real_cb: libbpf_sys::libbpf_print_fn_t = callback.as_ref().and(Some(outer_print_cb));
+    let real_cb: libbpf_sys::libbpf_print_fn_t = callback.is_some().then_some(outer_print_cb);
     std::mem::swap(&mut callback, &mut *PRINT_CB.lock().unwrap());
     unsafe { libbpf_sys::libbpf_set_print(real_cb) };
     callback
 }
 
 /// Return the current print callback and level.
-///
-/// # Examples
-///
-/// To temporarily suppress output:
-///
-/// ```
-/// use libbpf_rs::{get_print, set_print};
-///
-/// let prev = get_print();
-/// set_print(None);
-/// // do things quietly
-/// set_print(prev);
-/// ```
 pub fn get_print() -> Option<(PrintLevel, PrintCallback)> {
     *PRINT_CB.lock().unwrap()
 }

libbpf-rs/tests/test.rs

@@ -38,7 +38,7 @@ pub fn open_test_object(filename: &str) -> OpenObject {
     //     cargo test -- --nocapture
     //
     // To get all the output
-    builder.debug(true);
+    unsafe { builder.debug(true) };
     builder.open_file(obj_path).expect("failed to open object")
 }
 
@@ -112,8 +112,8 @@ fn test_object_build_from_memory() {
 #[test]
 fn test_object_load_invalid() {
     let empty_file = NamedTempFile::new().unwrap();
-    let _err = ObjectBuilder::default()
-        .debug(true)
+    let mut builder = ObjectBuilder::default();
+    let _err = unsafe { builder.debug(true) }
         .open_file(empty_file.path())
         .unwrap_err();
 }
@@ -488,7 +488,7 @@ fn test_object_reuse_pined_map() {
     // Reuse the pinned map
     let obj_path = get_test_object_path("runqslower.bpf.o");
     let mut builder = ObjectBuilder::default();
-    builder.debug(true);
+    unsafe { builder.debug(true) };
     let mut open_obj = builder.open_file(obj_path).expect("failed to open object");
 
     let start = open_obj.map_mut("start").expect("failed to find map");
@@ -1035,8 +1035,8 @@ fn test_object_link_files() {
         let () = linker.link().unwrap();
 
         // Check that we can load the resulting object file.
-        let _object = ObjectBuilder::default()
-            .debug(true)
+        let mut builder = ObjectBuilder::default();
+        let _object = unsafe { builder.debug(true) }
             .open_file(output_file.path())
             .unwrap();
     }

libbpf-rs/tests/test_print.rs

@@ -25,7 +25,9 @@ fn test_set_print() {
         }
     }
 
-    set_print(Some((PrintLevel::Debug, callback)));
+    unsafe {
+        set_print(Some((PrintLevel::Debug, callback)));
+    }
     // expect_err requires that OpenObject implement Debug, which it does not.
     let obj = ObjectBuilder::default().open_file("/dev/null");
     assert!(obj.is_err(), "Successfully loaded /dev/null?");
@@ -46,11 +48,15 @@ fn test_set_restore_print() {
         println!("two");
     }
 
-    set_print(Some((PrintLevel::Warn, callback1)));
+    unsafe {
+        set_print(Some((PrintLevel::Warn, callback1)));
+    }
     let prev = get_print();
     assert_eq!(prev, Some((PrintLevel::Warn, callback1 as PrintCallback)));
 
-    set_print(Some((PrintLevel::Debug, callback2)));
+    unsafe {
+        set_print(Some((PrintLevel::Debug, callback2)));
+    }
     let prev = get_print();
     assert_eq!(prev, Some((PrintLevel::Debug, callback2 as PrintCallback)));
 }
@@ -65,10 +71,12 @@ fn test_set_and_save_print() {
         println!("two");
     }
 
-    set_print(Some((PrintLevel::Warn, callback1)));
-    let prev = set_print(Some((PrintLevel::Debug, callback2)));
+    unsafe {
+        set_print(Some((PrintLevel::Warn, callback1)));
+    }
+    let prev = unsafe { set_print(Some((PrintLevel::Debug, callback2))) };
     assert_eq!(prev, Some((PrintLevel::Warn, callback1 as PrintCallback)));
 
-    let prev = set_print(None);
+    let prev = unsafe { set_print(None) };
     assert_eq!(prev, Some((PrintLevel::Debug, callback2 as PrintCallback)));
 }