Improve some type names and comments and other PR feedback.

include/openssl/arm_arch.h

@@ -104,6 +104,8 @@
 #define __ARM_MAX_ARCH__ 8
 #endif
 
+/* Keep in sync with src/cpu_feature/arm_linux/arm_linux.rs */
+
 /* ARMV7_NEON is true when a NEON unit is present in the current CPU. */
 #define ARMV7_NEON (1 << 0)
 

src/cpu_feature/arm_linux/arm_linux.rs

@@ -1,18 +1,18 @@
-use self::auxv::AuxvUnsignedLong;
+use self::auxv::Type;
 #[cfg(all(any(target_arch = "arm", target_arch = "aarch64"), target_os="linux"))]
-use self::auxv::NativeGetauxvalProvider;
-use self::auxv::{AT_HWCAP, AT_HWCAP2, GetauxvalProvider};
+use self::auxv::NativeProvider;
+use self::auxv::{AT_HWCAP, AT_HWCAP2, Provider};
 
-// Bits exposed in HWCAP and HWCAP2 auxv values
-const ARM_HWCAP2_AES: AuxvUnsignedLong = 1 << 0;
-const ARM_HWCAP2_PMULL: AuxvUnsignedLong = 1 << 1;
-const ARM_HWCAP2_SHA1: AuxvUnsignedLong = 1 << 2;
-const ARM_HWCAP2_SHA2: AuxvUnsignedLong = 1 << 3;
+// See /usr/include/asm/hwcap.h on an ARM installation for the source of these values
+const ARM_HWCAP2_AES: Type = 1 << 0;
+const ARM_HWCAP2_PMULL: Type = 1 << 1;
+const ARM_HWCAP2_SHA1: Type = 1 << 2;
+const ARM_HWCAP2_SHA2: Type = 1 << 3;
 
-const ARM_HWCAP_NEON: AuxvUnsignedLong = 1 << 12;
+const ARM_HWCAP_NEON: Type = 1 << 12;
 
 // Constants used in GFp_armcap_P
-// from include/openssl/arm_arch.h
+// Keep in sync with include/openssl/arm_arch.h
 const ARMV7_NEON: u32 = 1 << 0;
 // not a typo; there is no constant for 1 << 1
 const ARMV8_AES: u32 = 1 << 2;
@@ -30,49 +30,42 @@ extern "C" {
 #[cfg(all(any(target_arch = "arm", target_arch = "aarch64"),
     target_os="linux"))]
 pub fn arm_linux_set_cpu_features() {
-    let getauxval = NativeGetauxvalProvider{};
+    let getauxval = NativeProvider {};
 
     unsafe {
         GFp_armcap_P |= armcap_from_features(getauxval);
     }
 }
 
 /// returns a u32 with bits set for use in GFp_armcap_P
-fn armcap_from_features<G: GetauxvalProvider> (getauxval_provider: G) -> u32 {
-    let mut hwcap: AuxvUnsignedLong = 0;
+fn armcap_from_features<G: Provider> (getauxval: G) -> u32 {
+    let hwcap = getauxval.getauxval(AT_HWCAP).unwrap_or(0);
 
-    if let Ok(v) = getauxval_provider.getauxval(AT_HWCAP) {
-        hwcap = v;
-    }
-
-    // Matching OpenSSL, only report other features if NEON is present
+    // Matching OpenSSL, only report other features if NEON is present.
     let mut armcap: u32 = 0;
-    if hwcap & ARM_HWCAP_NEON > 0 {
+    if hwcap & ARM_HWCAP_NEON != 0 {
         armcap |= ARMV7_NEON;
 
-        let mut hwcap2 = 0;
-        if let Ok(v) = getauxval_provider.getauxval(AT_HWCAP2) {
-            hwcap2 = v;
-        }
+        let hwcap2 = getauxval.getauxval(AT_HWCAP2).unwrap_or(0);
 
         armcap |= armcap_for_hwcap2(hwcap2);
     }
 
     return armcap;
 }
 
-fn armcap_for_hwcap2(hwcap2: AuxvUnsignedLong) -> u32 {
+fn armcap_for_hwcap2(hwcap2: Type) -> u32 {
     let mut ret: u32 = 0;
-    if hwcap2 & ARM_HWCAP2_AES > 0 {
+    if hwcap2 & ARM_HWCAP2_AES != 0 {
         ret |= ARMV8_AES;
     }
-    if hwcap2 & ARM_HWCAP2_PMULL > 0 {
+    if hwcap2 & ARM_HWCAP2_PMULL != 0 {
         ret |= ARMV8_PMULL;
     }
-    if hwcap2 & ARM_HWCAP2_SHA1 > 0 {
+    if hwcap2 & ARM_HWCAP2_SHA1 != 0 {
         ret |= ARMV8_SHA1;
     }
-    if hwcap2 & ARM_HWCAP2_SHA2 > 0 {
+    if hwcap2 & ARM_HWCAP2_SHA2 != 0 {
         ret |= ARMV8_SHA256;
     }
 
@@ -89,31 +82,20 @@ mod tests {
         ARMV8_AES, ARMV8_PMULL, ARMV8_SHA1, ARMV8_SHA256, ARM_HWCAP2_AES,
         ARM_HWCAP2_PMULL, ARM_HWCAP2_SHA1, ARM_HWCAP2_SHA2,
         ARM_HWCAP_NEON};
-    #[cfg(target_os="linux")]
-    use super::auxv::NativeGetauxvalProvider;
-    use super::auxv::{AuxvUnsignedLong, GetauxvalError,
-        GetauxvalProvider, AT_HWCAP, AT_HWCAP2};
+    use super::auxv::{Type, GetauxvalError,
+        Provider, AT_HWCAP, AT_HWCAP2};
 
-    struct StubGetauxvalProvider {
-        auxv: HashMap<AuxvUnsignedLong, AuxvUnsignedLong>
+    struct StubGetauxval {
+        auxv: HashMap<Type, Type>
     }
 
-    impl GetauxvalProvider for StubGetauxvalProvider {
-        fn getauxval(&self, auxv_type: AuxvUnsignedLong)
-                -> Result<AuxvUnsignedLong, GetauxvalError> {
+    impl Provider for StubGetauxval {
+        fn getauxval(&self, auxv_type: Type)
+                -> Result<Type, GetauxvalError> {
             self.auxv.get(&auxv_type).map(|v| *v).ok_or(GetauxvalError::NotFound)
         }
     }
 
-    #[test]
-    #[cfg(target_os="linux")]
-    fn armcap_from_env_doesnt_crash() {
-        let getauxval = NativeGetauxvalProvider{};
-
-        // can't really say anything useful about its result
-        let _ = armcap_from_features(getauxval);
-    }
-
     #[test]
     fn armcap_bits_ok_neon_with_neon_getauxv_yields_neon_armcap() {
         do_armcap_bits_test(hwcap_neon_getauxv(), ARMV7_NEON);
@@ -124,7 +106,7 @@ mod tests {
         let mut auxv = HashMap::new();
         let _ = auxv.insert(AT_HWCAP, ARM_HWCAP_NEON);
         let _ = auxv.insert(AT_HWCAP2, ARM_HWCAP2_AES | ARM_HWCAP2_PMULL | ARM_HWCAP2_SHA1 | ARM_HWCAP2_SHA2);
-        let getauxv = StubGetauxvalProvider {
+        let getauxv = StubGetauxval {
             auxv: auxv
         };
 
@@ -137,18 +119,13 @@ mod tests {
         let mut auxv = HashMap::new();
         let _ = auxv.insert(AT_HWCAP, ARM_HWCAP_NEON);
         let _ = auxv.insert(AT_HWCAP2, ARM_HWCAP2_AES);
-        let getauxv = StubGetauxvalProvider {
+        let getauxv = StubGetauxval {
             auxv: auxv
         };
 
         do_armcap_bits_test(getauxv, ARMV7_NEON | ARMV8_AES);
     }
 
-    #[test]
-    fn armcap_for_hwcap2_zero_returns_zero() {
-        assert_eq!(0, super::armcap_for_hwcap2(0));
-    }
-
     #[test]
     fn armcap_for_hwcap2_all_hwcap2_returns_all_armcap() {
         assert_eq!(ARMV8_AES | ARMV8_PMULL | ARMV8_SHA1 | ARMV8_SHA256,
@@ -158,18 +135,18 @@ mod tests {
                                         | ARM_HWCAP2_SHA2));
     }
 
-    fn do_armcap_bits_test(getauxval: StubGetauxvalProvider,
+    fn do_armcap_bits_test(getauxval: StubGetauxval,
                            expected_armcap: u32) {
         assert_eq!(expected_armcap,
-            armcap_from_features::<StubGetauxvalProvider>(getauxval));
+            armcap_from_features::<StubGetauxval>(getauxval));
     }
 
-    fn hwcap_neon_getauxv() -> StubGetauxvalProvider {
+    fn hwcap_neon_getauxv() -> StubGetauxval {
         let mut auxv = HashMap::new();
         let _ = auxv.insert(AT_HWCAP,
                             ARM_HWCAP_NEON);
 
-        StubGetauxvalProvider {
+        StubGetauxval {
             auxv: auxv
         }
     }

src/cpu_feature/arm_linux/auxv.rs

@@ -1,16 +1,15 @@
 #[cfg(any(all(target_pointer_width = "32", test),
     all(target_pointer_width = "32", target_os = "linux")))]
-pub type AuxvUnsignedLong = u32;
+pub type Type = u32;
 #[cfg(any(all(target_pointer_width = "64", test),
     all(target_pointer_width = "64", target_os = "linux")))]
-pub type AuxvUnsignedLong = u64;
+pub type Type = u64;
 
 extern "C" {
     /// Invoke getauxval(3) if available. If it's not linked, or if invocation
     /// fails or the type is not found, sets success to false and returns 0.
     #[cfg(target_os="linux")]
-    pub fn getauxval_wrapper(auxv_type: AuxvUnsignedLong,
-                             success: *mut AuxvUnsignedLong) -> i32;
+    pub fn getauxval_wrapper(auxv_type: Type, success: *mut Type) -> i32;
 }
 
 #[derive(Debug, PartialEq)]
@@ -25,24 +24,23 @@ pub enum GetauxvalError {
     UnknownError
 }
 
-pub trait GetauxvalProvider {
+pub trait Provider {
     /// Look up an entry in the auxiliary vector. See getauxval(3) in glibc.
     /// Unfortunately, prior to glibc 2.19, getauxval() returns 0 without
     /// setting `errno` if the type is not found, so on such old systems
     /// this will return `Ok(0)` rather than `Err(GetauxvalError::NotFound)`.
-    fn getauxval(&self, auxv_type: AuxvUnsignedLong)
-        -> Result<AuxvUnsignedLong, GetauxvalError>;
+    fn getauxval(&self, auxv_type: Type) -> Result<Type, GetauxvalError>;
 }
 
 #[cfg(target_os="linux")]
-pub struct NativeGetauxvalProvider {}
+pub struct NativeProvider {}
 
 #[cfg(target_os="linux")]
-impl GetauxvalProvider for NativeGetauxvalProvider {
+impl Provider for NativeProvider {
     /// Returns Some if the native invocation succeeds and the requested type was
     /// found, otherwise None.
-    fn getauxval(&self, auxv_type: AuxvUnsignedLong)
-            -> Result<AuxvUnsignedLong, GetauxvalError> {
+    fn getauxval(&self, auxv_type: Type)
+            -> Result<Type, GetauxvalError> {
 
         let mut result = 0;
         unsafe {
@@ -59,32 +57,6 @@ impl GetauxvalProvider for NativeGetauxvalProvider {
 
 // from [linux]/include/uapi/linux/auxvec.h. First 32 bits of HWCAP
 // even on platforms where unsigned long is 64 bits.
-pub const AT_HWCAP: AuxvUnsignedLong = 16;
+pub const AT_HWCAP: Type = 16;
 // currently only used by powerpc and arm64 AFAICT
-pub const AT_HWCAP2: AuxvUnsignedLong = 26;
-
-#[cfg(test)]
-mod tests {
-    #[cfg(target_os="linux")]
-    use super::{AT_HWCAP, GetauxvalError, GetauxvalProvider,
-        NativeGetauxvalProvider};
-
-    #[test]
-    #[cfg(target_os="linux")]
-    fn test_getauxv_hwcap_linux_finds_hwcap() {
-        let native_getauxval = NativeGetauxvalProvider{};
-        let result = native_getauxval.getauxval(AT_HWCAP);
-        // there should be SOMETHING in the value
-        assert!(result.unwrap() > 0);
-    }
-
-    #[test]
-    #[cfg(target_os="linux")]
-    fn test_getauxv_hwcap_linux_doesnt_find_bogus_type() {
-        let native_getauxval = NativeGetauxvalProvider{};
-
-        // AT_NULL aka 0 is effectively the EOF for auxv, so it's never a valid type
-        assert_eq!(GetauxvalError::NotFound, native_getauxval.getauxval(0).unwrap_err());
-    }
-
-}
+pub const AT_HWCAP2: Type = 26;