Reduce the amount of code in unsafe blocks

src/attr.rs

@@ -22,15 +22,17 @@ pub enum AttrValue<'string> {
 }
 
 macro_rules! from_value {
-    ($value:expr => $string:expr) => {
-        match unsafe { raw::git_attr_value($value.map_or(ptr::null(), |v| v.as_ptr().cast())) } {
+    ($value:expr => $string:expr) => {{
+        let ptr = $value.map_or(ptr::null(), |v| v.as_ptr().cast());
+        let value = unsafe { raw::git_attr_value(ptr) };
+        match value {
             raw::GIT_ATTR_VALUE_TRUE => Self::True,
             raw::GIT_ATTR_VALUE_FALSE => Self::False,
             raw::GIT_ATTR_VALUE_STRING => $string,
             raw::GIT_ATTR_VALUE_UNSPECIFIED => Self::Unspecified,
             _ => unreachable!(),
         }
-    };
+    }};
 }
 
 impl<'string> AttrValue<'string> {

src/blame.rs

@@ -61,26 +61,22 @@ impl<'repo> Blame<'repo> {
 
     /// Gets the blame hunk at the given index.
     pub fn get_index(&self, index: usize) -> Option<BlameHunk<'_>> {
-        unsafe {
-            let ptr = raw::git_blame_get_hunk_byindex(self.raw(), index as u32);
-            if ptr.is_null() {
-                None
-            } else {
-                Some(BlameHunk::from_raw_const(ptr))
-            }
+        let ptr = unsafe { raw::git_blame_get_hunk_byindex(self.raw(), index as u32) };
+        if ptr.is_null() {
+            None
+        } else {
+            Some(unsafe { BlameHunk::from_raw_const(ptr) })
         }
     }
 
     /// Gets the hunk that relates to the given line number in the newest
     /// commit.
     pub fn get_line(&self, lineno: usize) -> Option<BlameHunk<'_>> {
-        unsafe {
-            let ptr = raw::git_blame_get_hunk_byline(self.raw(), lineno);
-            if ptr.is_null() {
-                None
-            } else {
-                Some(BlameHunk::from_raw_const(ptr))
-            }
+        let ptr = unsafe { raw::git_blame_get_hunk_byline(self.raw(), lineno) };
+        if ptr.is_null() {
+            None
+        } else {
+            Some(unsafe { BlameHunk::from_raw_const(ptr) })
         }
     }
 
@@ -181,12 +177,11 @@ impl<'blame> BlameHunk<'blame> {
     ///
     /// Note: `None` could be returned for non-unicode paths on Windows.
     pub fn path(&self) -> Option<&Path> {
-        unsafe {
-            if let Some(bytes) = crate::opt_bytes(self, (*self.raw).orig_path) {
-                Some(util::bytes2path(bytes))
-            } else {
-                None
-            }
+        let opt_bytes = unsafe { crate::opt_bytes(self, (*self.raw).orig_path) };
+        if let Some(bytes) = opt_bytes {
+            Some(util::bytes2path(bytes))
+        } else {
+            None
         }
     }
 

src/blob.rs

@@ -133,17 +133,16 @@ impl<'repo> Drop for BlobWriter<'repo> {
 
 impl<'repo> io::Write for BlobWriter<'repo> {
     fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
-        unsafe {
-            if let Some(f) = (*self.raw).write {
-                let res = f(self.raw, buf.as_ptr() as *const _, buf.len());
-                if res < 0 {
-                    Err(io::Error::new(io::ErrorKind::Other, "Write error"))
-                } else {
-                    Ok(buf.len())
-                }
+        let write_cb = unsafe { (*self.raw).write };
+        if let Some(f) = write_cb {
+            let res = f(self.raw, buf.as_ptr() as *const _, buf.len());
+            if res < 0 {
+                Err(io::Error::new(io::ErrorKind::Other, "Write error"))
             } else {
-                Err(io::Error::new(io::ErrorKind::Other, "no write callback"))
+                Ok(buf.len())
             }
+        } else {
+            Err(io::Error::new(io::ErrorKind::Other, "no write callback"))
         }
     }
     fn flush(&mut self) -> io::Result<()> {

src/branch.rs

@@ -119,8 +119,8 @@ impl<'repo> Branch<'repo> {
                 self.get().raw(),
                 upstream_name
             ));
-            Ok(())
         }
+        Ok(())
     }
 }
 

src/cert.rs

@@ -92,81 +92,78 @@ impl<'a> Cert<'a> {
 
     fn cast<T>(&self, kind: raw::git_cert_t) -> Option<&T> {
         assert_eq!(mem::size_of::<Cert<'a>>(), mem::size_of::<T>());
-        unsafe {
-            if kind == (*self.raw).cert_type {
-                Some(&*(self as *const Cert<'a> as *const T))
-            } else {
-                None
-            }
+        let current_kind = unsafe { (*self.raw).cert_type };
+        if kind == current_kind {
+            Some(unsafe { &*(self as *const Cert<'a> as *const T) })
+        } else {
+            None
         }
     }
 }
 
 impl<'a> CertHostkey<'a> {
     /// Returns the md5 hash of the hostkey, if available.
     pub fn hash_md5(&self) -> Option<&[u8; 16]> {
-        unsafe {
-            if (*self.raw).kind as u32 & raw::GIT_CERT_SSH_MD5 as u32 == 0 {
-                None
-            } else {
-                Some(&(*self.raw).hash_md5)
-            }
+        let kind = unsafe { (*self.raw).kind };
+        if kind as u32 & raw::GIT_CERT_SSH_MD5 as u32 == 0 {
+            None
+        } else {
+            Some(unsafe { &(*self.raw).hash_md5 })
         }
     }
 
     /// Returns the SHA-1 hash of the hostkey, if available.
     pub fn hash_sha1(&self) -> Option<&[u8; 20]> {
-        unsafe {
-            if (*self.raw).kind as u32 & raw::GIT_CERT_SSH_SHA1 as u32 == 0 {
-                None
-            } else {
-                Some(&(*self.raw).hash_sha1)
-            }
+        let kind = unsafe { (*self.raw).kind };
+        if kind as u32 & raw::GIT_CERT_SSH_SHA1 as u32 == 0 {
+            None
+        } else {
+            Some(unsafe { &(*self.raw).hash_sha1 })
         }
     }
 
     /// Returns the SHA-256 hash of the hostkey, if available.
     pub fn hash_sha256(&self) -> Option<&[u8; 32]> {
-        unsafe {
-            if (*self.raw).kind as u32 & raw::GIT_CERT_SSH_SHA256 as u32 == 0 {
-                None
-            } else {
-                Some(&(*self.raw).hash_sha256)
-            }
+        let kind = unsafe { (*self.raw).kind };
+        if kind as u32 & raw::GIT_CERT_SSH_SHA256 as u32 == 0 {
+            None
+        } else {
+            Some(unsafe { &(*self.raw).hash_sha256 })
         }
     }
 
     /// Returns the raw host key.
     pub fn hostkey(&self) -> Option<&[u8]> {
-        unsafe {
-            if (*self.raw).kind & raw::GIT_CERT_SSH_RAW == 0 {
-                return None;
-            }
-            Some(slice::from_raw_parts(
+        let kind = unsafe { (*self.raw).kind };
+        if kind & raw::GIT_CERT_SSH_RAW == 0 {
+            return None;
+        }
+        Some(unsafe {
+            slice::from_raw_parts(
                 (*self.raw).hostkey as *const u8,
                 (*self.raw).hostkey_len as usize,
-            ))
-        }
+            )
+        })
     }
 
     /// Returns the type of the host key.
     pub fn hostkey_type(&self) -> Option<SshHostKeyType> {
-        unsafe {
-            if (*self.raw).kind & raw::GIT_CERT_SSH_RAW == 0 {
-                return None;
-            }
-            let t = match (*self.raw).raw_type {
-                raw::GIT_CERT_SSH_RAW_TYPE_UNKNOWN => SshHostKeyType::Unknown,
-                raw::GIT_CERT_SSH_RAW_TYPE_RSA => SshHostKeyType::Rsa,
-                raw::GIT_CERT_SSH_RAW_TYPE_DSS => SshHostKeyType::Dss,
-                raw::GIT_CERT_SSH_RAW_TYPE_KEY_ECDSA_256 => SshHostKeyType::Ecdsa256,
-                raw::GIT_CERT_SSH_RAW_TYPE_KEY_ECDSA_384 => SshHostKeyType::Ecdsa384,
-                raw::GIT_CERT_SSH_RAW_TYPE_KEY_ECDSA_521 => SshHostKeyType::Ecdsa521,
-                raw::GIT_CERT_SSH_RAW_TYPE_KEY_ED25519 => SshHostKeyType::Ed255219,
-                t => panic!("unexpected host key type {:?}", t),
-            };
-            Some(t)
+        let kind = unsafe { (*self.raw).kind };
+        if kind & raw::GIT_CERT_SSH_RAW == 0 {
+            return None;
         }
+        let raw_type = unsafe { (*self.raw).raw_type };
+        let t = match raw_type {
+            raw::GIT_CERT_SSH_RAW_TYPE_UNKNOWN => SshHostKeyType::Unknown,
+            raw::GIT_CERT_SSH_RAW_TYPE_RSA => SshHostKeyType::Rsa,
+            raw::GIT_CERT_SSH_RAW_TYPE_DSS => SshHostKeyType::Dss,
+            raw::GIT_CERT_SSH_RAW_TYPE_KEY_ECDSA_256 => SshHostKeyType::Ecdsa256,
+            raw::GIT_CERT_SSH_RAW_TYPE_KEY_ECDSA_384 => SshHostKeyType::Ecdsa384,
+            raw::GIT_CERT_SSH_RAW_TYPE_KEY_ECDSA_521 => SshHostKeyType::Ecdsa521,
+            raw::GIT_CERT_SSH_RAW_TYPE_KEY_ED25519 => SshHostKeyType::Ed255219,
+            t => panic!("unexpected host key type {:?}", t),
+        };
+        Some(t)
     }
 }
 

src/cherrypick.rs

@@ -44,29 +44,35 @@ impl<'cb> CherrypickOptions<'cb> {
 
     /// Obtain the raw struct
     pub fn raw(&mut self) -> raw::git_cherrypick_options {
+        let mut checkout_opts: raw::git_checkout_options = unsafe { mem::zeroed() };
         unsafe {
-            let mut checkout_opts: raw::git_checkout_options = mem::zeroed();
             raw::git_checkout_init_options(&mut checkout_opts, raw::GIT_CHECKOUT_OPTIONS_VERSION);
-            if let Some(ref mut cb) = self.checkout_builder {
-                cb.configure(&mut checkout_opts);
-            }
+        }
+        if let Some(ref mut cb) = self.checkout_builder {
+            unsafe { cb.configure(&mut checkout_opts) };
+        }
 
-            let mut merge_opts: raw::git_merge_options = mem::zeroed();
+        let mut merge_opts: raw::git_merge_options = unsafe { mem::zeroed() };
+        unsafe {
             raw::git_merge_init_options(&mut merge_opts, raw::GIT_MERGE_OPTIONS_VERSION);
-            if let Some(ref opts) = self.merge_opts {
+        }
+        if let Some(ref opts) = self.merge_opts {
+            unsafe {
                 ptr::copy(opts.raw(), &mut merge_opts, 1);
             }
+        }
 
-            let mut cherrypick_opts: raw::git_cherrypick_options = mem::zeroed();
+        let mut cherrypick_opts: raw::git_cherrypick_options = unsafe { mem::zeroed() };
+        unsafe {
             raw::git_cherrypick_init_options(
                 &mut cherrypick_opts,
                 raw::GIT_CHERRYPICK_OPTIONS_VERSION,
             );
-            cherrypick_opts.mainline = self.mainline;
-            cherrypick_opts.checkout_opts = checkout_opts;
-            cherrypick_opts.merge_opts = merge_opts;
-
-            cherrypick_opts
         }
+        cherrypick_opts.mainline = self.mainline;
+        cherrypick_opts.checkout_opts = checkout_opts;
+        cherrypick_opts.merge_opts = merge_opts;
+
+        cherrypick_opts
     }
 }

src/commit.rs

@@ -175,12 +175,9 @@ impl<'repo> Commit<'repo> {
     /// The second element is the offset, in minutes, of the time zone of the
     /// committer's preferred time zone.
     pub fn time(&self) -> Time {
-        unsafe {
-            Time::new(
-                raw::git_commit_time(&*self.raw) as i64,
-                raw::git_commit_time_offset(&*self.raw) as i32,
-            )
-        }
+        let time = unsafe { raw::git_commit_time(&*self.raw) } as i64;
+        let offset = unsafe { raw::git_commit_time_offset(&*self.raw) } as i32;
+        Time::new(time, offset)
     }
 
     /// Creates a new iterator over the parents of this commit.
@@ -309,13 +306,11 @@ impl<'repo> Commit<'repo> {
     ///
     /// Use the `parent_ids` iterator to return an iterator over all parents.
     pub fn parent_id(&self, i: usize) -> Result<Oid, Error> {
-        unsafe {
-            let id = raw::git_commit_parent_id(self.raw, i as libc::c_uint);
-            if id.is_null() {
-                Err(Error::from_str("parent index out of bounds"))
-            } else {
-                Ok(Binding::from_raw(id))
-            }
+        let id = unsafe { raw::git_commit_parent_id(self.raw, i as libc::c_uint) };
+        if id.is_null() {
+            Err(Error::from_str("parent index out of bounds"))
+        } else {
+            Ok(unsafe { Binding::from_raw(id) })
         }
     }
 

src/config.rs

@@ -163,8 +163,8 @@ impl Config {
                 ptr::null(),
                 force
             ));
-            Ok(())
         }
+        Ok(())
     }
 
     /// Delete a config variable from the config file with the highest level
@@ -173,8 +173,8 @@ impl Config {
         let name = CString::new(name)?;
         unsafe {
             try_call!(raw::git_config_delete_entry(self.raw, name));
-            Ok(())
         }
+        Ok(())
     }
 
     /// Remove multivar config variables in the config file with the highest level (usually the
@@ -334,18 +334,16 @@ impl Config {
     /// ```
     pub fn entries(&self, glob: Option<&str>) -> Result<ConfigEntries<'_>, Error> {
         let mut ret = ptr::null_mut();
-        unsafe {
-            match glob {
-                Some(s) => {
-                    let s = CString::new(s)?;
-                    try_call!(raw::git_config_iterator_glob_new(&mut ret, &*self.raw, s));
-                }
-                None => {
-                    try_call!(raw::git_config_iterator_new(&mut ret, &*self.raw));
-                }
+        match glob {
+            Some(s) => {
+                let s = CString::new(s)?;
+                unsafe { try_call!(raw::git_config_iterator_glob_new(&mut ret, &*self.raw, s)) };
+            }
+            None => {
+                unsafe { try_call!(raw::git_config_iterator_new(&mut ret, &*self.raw)) };
             }
-            Ok(Binding::from_raw(ret))
         }
+        Ok(unsafe { Binding::from_raw(ret) })
     }
 
     /// Iterate over the values of a multivar
@@ -558,7 +556,7 @@ impl<'cfg> ConfigEntry<'cfg> {
 
     /// Gets the configuration level of this entry.
     pub fn level(&self) -> ConfigLevel {
-        unsafe { ConfigLevel::from_raw((*self.raw).level) }
+        ConfigLevel::from_raw(unsafe { (*self.raw).level })
     }
 
     /// Depth of includes where this variable was found
@@ -606,14 +604,14 @@ impl<'cfg> ConfigEntries<'cfg> {
         drop(self.current.take());
         unsafe {
             try_call_iter!(raw::git_config_next(&mut raw, self.raw));
-            let entry = ConfigEntry {
-                owned: false,
-                raw,
-                _marker: marker::PhantomData,
-            };
-            self.current = Some(entry);
-            Some(Ok(self.current.as_ref().unwrap()))
         }
+        let entry = ConfigEntry {
+            owned: false,
+            raw,
+            _marker: marker::PhantomData,
+        };
+        self.current = Some(entry);
+        Some(Ok(self.current.as_ref().unwrap()))
     }
 
     /// Calls the given closure for each remaining entry in the iterator.