Merge pull request #156 from gtsiam/panic-safety

ruabmbua · web-flow · commit 009df084a952 · 2025-06-09T13:22:58.000+02:00
Improved panic safety
diff --git a/build.rs b/build.rs
@@ -49,72 +49,53 @@ fn compile_linux() {
     // First check the features enabled for the crate.
     // Only one linux backend should be enabled at a time.
 
-    let avail_backends: [(&'static str, Box<dyn Fn()>); 6] = [
-        (
-            "LINUX_STATIC_HIDRAW",
-            Box::new(|| {
-                let mut config = cc::Build::new();
-                println!("cargo:rerun-if-changed=etc/hidapi/linux/hid.c");
-                config
-                    .file("etc/hidapi/linux/hid.c")
-                    .include("etc/hidapi/hidapi");
-                pkg_config::probe_library("libudev").expect("Unable to find libudev");
-                config.compile("libhidapi.a");
-                println!("cargo:rustc-cfg=hidapi");
-            }),
-        ),
-        (
-            "LINUX_STATIC_LIBUSB",
-            Box::new(|| {
-                let mut config = cc::Build::new();
-                println!("cargo:rerun-if-changed=etc/hidapi/linux/hid.c");
-                config
-                    .file("etc/hidapi/libusb/hid.c")
-                    .include("etc/hidapi/hidapi");
-                let lib =
-                    pkg_config::find_library("libusb-1.0").expect("Unable to find libusb-1.0");
-                for path in lib.include_paths {
-                    config.include(
-                        path.to_str()
-                            .expect("Failed to convert include path to str"),
-                    );
-                }
-                config.compile("libhidapi.a");
-                println!("cargo:rustc-cfg=libusb");
-                println!("cargo:rustc-cfg=hidapi");
-            }),
-        ),
-        (
-            "LINUX_SHARED_HIDRAW",
-            Box::new(|| {
-                pkg_config::probe_library("hidapi-hidraw").expect("Unable to find hidapi-hidraw");
-                println!("cargo:rustc-cfg=hidapi");
-            }),
-        ),
-        (
-            "LINUX_SHARED_LIBUSB",
-            Box::new(|| {
-                pkg_config::probe_library("libusb-1.0").expect("Unable to find libusb-1.0");
-                pkg_config::probe_library("hidapi-libusb").expect("Unable to find hidapi-libusb");
-                println!("cargo:rustc-cfg=libusb");
-                println!("cargo:rustc-cfg=hidapi");
-            }),
-        ),
-        (
-            "LINUX_NATIVE",
-            Box::new(|| {
-                // The udev crate takes care of finding its library
-            }),
-        ),
-        (
-            "LINUX_NATIVE_BASIC_UDEV",
-            Box::new(|| {
-                // Enable `feature="linux-native"` to reuse the existing
-                // linux-native code. It is considered an error in
-                // basic-udev if this fails to compile.
-                println!("cargo:rustc-cfg=feature=\"linux-native\"");
-            }),
-        ),
+    let avail_backends: [(&'static str, &dyn Fn()); 6] = [
+        ("LINUX_STATIC_HIDRAW", &|| {
+            let mut config = cc::Build::new();
+            println!("cargo:rerun-if-changed=etc/hidapi/linux/hid.c");
+            config
+                .file("etc/hidapi/linux/hid.c")
+                .include("etc/hidapi/hidapi");
+            pkg_config::probe_library("libudev").expect("Unable to find libudev");
+            config.compile("libhidapi.a");
+            println!("cargo:rustc-cfg=hidapi");
+        }),
+        ("LINUX_STATIC_LIBUSB", &|| {
+            let mut config = cc::Build::new();
+            println!("cargo:rerun-if-changed=etc/hidapi/linux/hid.c");
+            config
+                .file("etc/hidapi/libusb/hid.c")
+                .include("etc/hidapi/hidapi");
+            let lib = pkg_config::find_library("libusb-1.0").expect("Unable to find libusb-1.0");
+            for path in lib.include_paths {
+                config.include(
+                    path.to_str()
+                        .expect("Failed to convert include path to str"),
+                );
+            }
+            config.compile("libhidapi.a");
+            println!("cargo:rustc-cfg=libusb");
+            println!("cargo:rustc-cfg=hidapi");
+        }),
+        ("LINUX_SHARED_HIDRAW", &|| {
+            pkg_config::probe_library("hidapi-hidraw").expect("Unable to find hidapi-hidraw");
+            println!("cargo:rustc-cfg=hidapi");
+        }),
+        ("LINUX_SHARED_LIBUSB", &|| {
+            pkg_config::probe_library("libusb-1.0").expect("Unable to find libusb-1.0");
+            pkg_config::probe_library("hidapi-libusb").expect("Unable to find hidapi-libusb");
+            println!("cargo:rustc-cfg=libusb");
+            println!("cargo:rustc-cfg=hidapi");
+        }),
+        ("LINUX_NATIVE", &|| {
+            // The udev crate takes care of finding its library
+        }),
+        ("LINUX_NATIVE_BASIC_UDEV", &|| {
+            // Enable `feature="linux-native"` to reuse the existing
+            // linux-native code. It is considered an error in
+            // basic-udev if this fails to compile.
+            println!("cargo:rustc-cfg=feature=\"linux-native\"");
+        }),
     ];
 
     let mut backends = avail_backends
@@ -155,31 +136,24 @@ fn compile_illumos() {
     // First check the features enabled for the crate.
     // Only one illumos backend should be enabled at a time.
 
-    let avail_backends: [(&'static str, Box<dyn Fn()>); 2] = [
-        (
-            "ILLUMOS_STATIC_LIBUSB",
-            Box::new(|| {
-                let mut config = cc::Build::new();
-                config
-                    .file("etc/hidapi/libusb/hid.c")
-                    .include("etc/hidapi/hidapi");
-                let lib =
-                    pkg_config::find_library("libusb-1.0").expect("Unable to find libusb-1.0");
-                for path in lib.include_paths {
-                    config.include(
-                        path.to_str()
-                            .expect("Failed to convert include path to str"),
-                    );
-                }
-                config.compile("libhidapi.a");
-            }),
-        ),
-        (
-            "ILLUMOS_SHARED_LIBUSB",
-            Box::new(|| {
-                pkg_config::probe_library("hidapi-libusb").expect("Unable to find hidapi-libusb");
-            }),
-        ),
+    let avail_backends: [(&'static str, &dyn Fn()); 2] = [
+        ("ILLUMOS_STATIC_LIBUSB", &|| {
+            let mut config = cc::Build::new();
+            config
+                .file("etc/hidapi/libusb/hid.c")
+                .include("etc/hidapi/hidapi");
+            let lib = pkg_config::find_library("libusb-1.0").expect("Unable to find libusb-1.0");
+            for path in lib.include_paths {
+                config.include(
+                    path.to_str()
+                        .expect("Failed to convert include path to str"),
+                );
+            }
+            config.compile("libhidapi.a");
+        }),
+        ("ILLUMOS_SHARED_LIBUSB", &|| {
+            pkg_config::probe_library("hidapi-libusb").expect("Unable to find hidapi-libusb");
+        }),
     ];
 
     let mut backends = avail_backends
diff --git a/examples/co2mon.rs b/examples/co2mon.rs
@@ -78,7 +78,7 @@ fn decode_buf(buf: Packet) -> CO2Result {
                 CO2Result::Concentration(val)
             }
         }
-        _ => CO2Result::Unknown(res[0], val),
+        _ => CO2Result::Unknown(kind, val),
     }
 }
 
@@ -113,7 +113,7 @@ fn main() -> Result<(), HidError> {
         match decode_buf(buf) {
             CO2Result::Temperature(val) => println!("Temp:\t{val}"),
             CO2Result::Concentration(val) => println!("Conc:\t{val}"),
-            CO2Result::Unknown(..) => (),
+            CO2Result::Unknown(kind, val) => eprintln!("Unknown({kind}):\t{val}"),
             CO2Result::Error(msg) => {
                 return Err(invalid_data_err(msg));
             }
diff --git a/src/lib.rs b/src/lib.rs
@@ -126,21 +126,44 @@ cfg_if! {
 pub type HidResult<T> = Result<T, HidError>;
 pub const MAX_REPORT_DESCRIPTOR_SIZE: usize = 4096;
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+struct ContextState {
+    device_discovery: bool,
+    init_state: InitState,
+}
+
 enum InitState {
     NotInit,
-    Init { enumerate: bool },
+    Init,
 }
 
-static INIT_STATE: Mutex<InitState> = Mutex::new(InitState::NotInit);
+/// Global state to coordinate backing C library global context management.
+static CONTEXT_STATE: Mutex<ContextState> = Mutex::new(ContextState {
+    device_discovery: true,
+    init_state: InitState::NotInit,
+});
+
+/// `hidapi` context.
+///
+/// The `hidapi` C library is lazily initialized when creating the first instance,
+/// and never deinitialized. Therefore, it is allowed to create multiple `HidApi`
+/// instances.
+///
+/// Each instance has its own device list cache.
+pub struct HidApi {
+    device_list: Vec<DeviceInfo>,
+}
 
-fn lazy_init(do_enumerate: bool) -> HidResult<()> {
-    let mut init_state = INIT_STATE.lock().unwrap();
+impl HidApi {
+    /// Create a new hidapi context.
+    ///
+    /// Will also initialize the currently available device list if device discovery has not already
+    /// been [disabled](Self::disable_device_discovery).
+    pub fn new() -> HidResult<Self> {
+        let mut state = CONTEXT_STATE.lock().unwrap();
 
-    match *init_state {
-        InitState::NotInit => {
+        if let InitState::NotInit = state.init_state {
             #[cfg(all(libusb, not(target_os = "freebsd")))]
-            if !do_enumerate {
+            if !state.device_discovery {
                 // Do not scan for devices in libusb_init()
                 // Must be set before calling it.
                 // This is needed on Android, where access to USB devices is limited
@@ -158,42 +181,8 @@ fn lazy_init(do_enumerate: bool) -> HidResult<()> {
                 ffi::macos::hid_darwin_set_open_exclusive(0)
             }
 
-            *init_state = InitState::Init {
-                enumerate: do_enumerate,
-            }
+            state.init_state = InitState::Init;
         }
-        InitState::Init { enumerate } => {
-            if enumerate != do_enumerate {
-                panic!("Trying to initialize hidapi with enumeration={}, but it is already initialized with enumeration={}.", do_enumerate, enumerate)
-            }
-        }
-    }
-
-    Ok(())
-}
-
-/// `hidapi` context.
-///
-/// The `hidapi` C library is lazily initialized when creating the first instance,
-/// and never deinitialized. Therefore, it is allowed to create multiple `HidApi`
-/// instances.
-///
-/// Each instance has its own device list cache.
-pub struct HidApi {
-    device_list: Vec<DeviceInfo>,
-}
-
-impl HidApi {
-    /// Create a new hidapi context.
-    ///
-    /// Will also initialize the currently available device list.
-    ///
-    /// # Panics
-    ///
-    /// Panics if hidapi is already initialized in "without enumerate" mode
-    /// (i.e. if `new_without_enumerate()` has been called before).
-    pub fn new() -> HidResult<Self> {
-        lazy_init(true)?;
 
         let mut api = HidApi {
             device_list: Vec::with_capacity(8),
@@ -202,20 +191,47 @@ impl HidApi {
         Ok(api)
     }
 
-    /// Create a new hidapi context, in "do not enumerate" mode.
+    /// Disable device discovery on context creation.
     ///
-    /// This is needed on Android, where access to USB device enumeration is limited.
+    /// This may be necessary on Android, where access to USB device enumeration is limited.
     ///
     /// # Panics
     ///
-    /// Panics if hidapi is already initialized in "do enumerate" mode
-    /// (i.e. if `new()` has been called before).
-    pub fn new_without_enumerate() -> HidResult<Self> {
-        lazy_init(false)?;
+    /// Panics if an hidapi context has already been initialized with device discovery.
+    ///
+    /// <section class="warning">
+    ///
+    /// Avoid using this in library code, as it is an inherently global operation.
+    ///
+    /// This function is intended to be called by code that knows the environment it is running in.
+    /// Usually this means application code either directly, or through another abstraction.
+    ///
+    /// </section>
+    pub fn disable_device_discovery() {
+        let mut state = CONTEXT_STATE.lock().unwrap();
+
+        if let InitState::NotInit = state.init_state {
+            state.device_discovery = false; // Only disable device discovery before init.
+        } else if state.device_discovery {
+            core::mem::drop(state); // Make sure we don't poison the lock when panicking.
+            panic!("Cannot disable device discovery after HidApi has been initialized");
+        }
+    }
 
-        Ok(HidApi {
-            device_list: Vec::new(),
-        })
+    /// Create a new hidapi context, after disabling discovery. Please avoid using this function in
+    /// library code, because it forces all instances of HidApi to disable device discovery.
+    ///
+    /// See [`HidApi::disable_device_discovery()`].
+    ///
+    /// # Panics
+    ///
+    /// Panics if an hidapi context has already been initialized with device discovery.
+    #[deprecated(
+        note = "Please use only `HidApi::new()` in library code. Application code should disable device discovery explicitly."
+    )]
+    pub fn new_without_enumerate() -> HidResult<Self> {
+        Self::disable_device_discovery();
+        Self::new()
     }
 
     /// Refresh devices list and information about them (to access them use

Original file line number	Diff line number	Diff line change
`@@ -78,7 +78,7 @@ fn decode_buf(buf: Packet) -> CO2Result {`
`78`	`78`	`CO2Result::Concentration(val)`
`79`	`79`	`}`
`80`	`80`	`}`
`81`		`- _ => CO2Result::Unknown(res[0], val),`
	`81`	`+ _ => CO2Result::Unknown(kind, val),`
`82`	`82`	`}`
`83`	`83`	`}`
`84`	`84`
`@@ -113,7 +113,7 @@ fn main() -> Result<(), HidError> {`
`113`	`113`	`match decode_buf(buf) {`
`114`	`114`	`CO2Result::Temperature(val) => println!("Temp:\t{val}"),`
`115`	`115`	`CO2Result::Concentration(val) => println!("Conc:\t{val}"),`
`116`		`- CO2Result::Unknown(..) => (),`
	`116`	`+ CO2Result::Unknown(kind, val) => eprintln!("Unknown({kind}):\t{val}"),`
`117`	`117`	`CO2Result::Error(msg) => {`
`118`	`118`	`return Err(invalid_data_err(msg));`
`119`	`119`	`}`