Repository: GuillaumeGomez/sysinfo
Pull request #412 (2021-01-25 20:42:22)

Change return type of SystemExt::get_physical_core_count from "usize" into "Option<usize>"
Follow-up of #408.

@mjarkk: After thinking about it for a while, I think it makes more sense to return `None` instead of `0` when we failed to get the number. At least we make it explicit which makes more sense.


Patch:
diff --git a/src/apple/system.rs b/src/apple/system.rs
index 84fb886..0705c6d 100644
--- a/src/apple/system.rs
+++ b/src/apple/system.rs
@@ -347,7 +347,7 @@ impl SystemExt for System {
         &self.processors
     }
 
-    fn get_physical_core_count(&self) -> usize {
+    fn get_physical_core_count(&self) -> Option<usize> {
         let mut physical_core_count = 0;
 
         if unsafe {
@@ -357,9 +357,9 @@ impl SystemExt for System {
                 &mut physical_core_count as *mut usize as *mut c_void,
             )
         } {
-            physical_core_count
+            Some(physical_core_count)
         } else {
-            0
+            None
         }
     }
 
diff --git a/src/linux/processor.rs b/src/linux/processor.rs
index b869e57..39776aa 100644
--- a/src/linux/processor.rs
+++ b/src/linux/processor.rs
@@ -261,13 +261,13 @@ pub fn get_cpu_frequency(cpu_core_index: usize) -> u64 {
         .unwrap_or_default()
 }
 
-pub fn get_physical_core_count() -> usize {
+pub fn get_physical_core_count() -> Option<usize> {
     let mut s = String::new();
     if File::open("/proc/cpuinfo")
         .and_then(|mut f| f.read_to_string(&mut s))
         .is_err()
     {
-        return 0;
+        return None;
     }
 
     let mut core_ids_and_physical_ids: HashSet<String> = HashSet::new();
@@ -294,7 +294,7 @@ pub fn get_physical_core_count() -> usize {
         }
     }
 
-    core_ids_and_physical_ids.len()
+    Some(core_ids_and_physical_ids.len())
 }
 
 /// Returns the brand/vendor string for the first CPU (which should be the same for all CPUs).
diff --git a/src/linux/system.rs b/src/linux/system.rs
index 37d1ed7..57dac04 100644
--- a/src/linux/system.rs
+++ b/src/linux/system.rs
@@ -421,7 +421,7 @@ impl SystemExt for System {
         &self.processors
     }
 
-    fn get_physical_core_count(&self) -> usize {
+    fn get_physical_core_count(&self) -> Option<usize> {
         get_physical_core_count()
     }
 
diff --git a/src/traits.rs b/src/traits.rs
index a912bbd..286755f 100644
--- a/src/traits.rs
+++ b/src/traits.rs
@@ -733,7 +733,7 @@ pub trait SystemExt: Sized + Debug + Default {
     /// ```
     fn get_processors(&self) -> &[Processor];
 
-    /// Returns the number of physical cores on the processor.
+    /// Returns the number of physical cores on the processor or `None` if it couldn't get it.
     ///
     /// In case there are multiple CPUs, it will combine the physical core count of all the CPUs.
     ///
@@ -743,9 +743,9 @@ pub trait SystemExt: Sized + Debug + Default {
     /// use sysinfo::{ProcessorExt, System, SystemExt};
     ///
     /// let s = System::new();
-    /// println!("{}", s.get_physical_core_count());
+    /// println!("{:?}", s.get_physical_core_count());
     /// ```
-    fn get_physical_core_count(&self) -> usize;
+    fn get_physical_core_count(&self) -> Option<usize>;
 
     /// Returns the RAM size in kB.
     ///
diff --git a/src/unknown/system.rs b/src/unknown/system.rs
index 9b9b12a..d6026d0 100644
--- a/src/unknown/system.rs
+++ b/src/unknown/system.rs
@@ -76,8 +76,8 @@ impl SystemExt for System {
         &[]
     }
 
-    fn get_physical_core_count(&self) -> usize {
-        0
+    fn get_physical_core_count(&self) -> Option<usize> {
+        None
     }
 
     fn get_total_memory(&self) -> u64 {
diff --git a/src/windows/processor.rs b/src/windows/processor.rs
index f8726a2..1b775d9 100644
--- a/src/windows/processor.rs
+++ b/src/windows/processor.rs
@@ -415,7 +415,7 @@ pub fn get_frequencies(nb_processors: usize) -> Vec<u64> {
     }
 }
 
-pub fn get_physical_core_count() -> usize {
+pub fn get_physical_core_count() -> Option<usize> {
     // we cannot use the number of processors here to pre calculate the buf size
     // GetLogicalProcessorInformationEx with RelationProcessorCore passed to it not only returns the logical cores but also numa nodes
     //
@@ -425,21 +425,22 @@ pub fn get_physical_core_count() -> usize {
     unsafe { GetLogicalProcessorInformationEx(0, null_mut(), &mut needed_size) };
     let size = mem::size_of::<SYSTEM_LOGICAL_PROCESSOR_INFORMATION>() as u32;
     if needed_size == 0 || needed_size < size || needed_size % size != 0 {
-        return 0;
+        return None;
     }
 
     let count = needed_size / size;
-    let mut buf = Vec::with_capacity(count as usize);
+    let mut buf = Vec::with_capacity(count as _);
 
-    let result = unsafe { GetLogicalProcessorInformationEx(0, buf.as_mut_ptr(), &mut needed_size) };
-    if result == 0 {
-        return 0;
+    if unsafe { GetLogicalProcessorInformationEx(0, buf.as_mut_ptr(), &mut needed_size) } == 0 {
+        return None;
     }
 
     unsafe {
-        buf.set_len(count as usize);
+        buf.set_len(count as _);
     }
-    buf.iter()
-        .filter(|proc_info| proc_info.Relationship == 0) // Only get the physical cores
-        .count()
+    Some(
+        buf.iter()
+            .filter(|proc_info| proc_info.Relationship == 0) // Only get the physical cores
+            .count(),
+    )
 }
diff --git a/src/windows/system.rs b/src/windows/system.rs
index 9a56593..754ab0b 100644
--- a/src/windows/system.rs
+++ b/src/windows/system.rs
@@ -326,7 +326,7 @@ impl SystemExt for System {
         &self.processors
     }
 
-    fn get_physical_core_count(&self) -> usize {
+    fn get_physical_core_count(&self) -> Option<usize> {
         get_physical_core_count()
     }
 


Test patch:
diff --git a/tests/processor.rs b/tests/processor.rs
index f551c89..df043b6 100644
--- a/tests/processor.rs
+++ b/tests/processor.rs
@@ -24,7 +24,7 @@ fn test_physical_core_numbers() {
     use sysinfo::SystemExt;
 
     let s = sysinfo::System::new();
-    assert_ne!(s.get_physical_core_count(), 0);
-    let s = sysinfo::System::new_all();
-    assert_ne!(s.get_physical_core_count(), 0);
+    let count = s.get_physical_core_count();
+    assert_ne!(count, None);
+    assert!(count.unwrap() > 0);
 }