Merge pull request #1043 from ehuss/callback-errors

Consistently use the error message from a callback error.

src/error.rs

@@ -1,7 +1,7 @@
 use libc::c_int;
 use std::env::JoinPathsError;
 use std::error;
-use std::ffi::{CStr, NulError};
+use std::ffi::{CStr, CString, NulError};
 use std::fmt;
 use std::str;
 
@@ -350,6 +350,17 @@ impl Error {
     pub fn message(&self) -> &str {
         &self.message
     }
+
+    /// A low-level convenience to call [`raw::git_error_set_str`] with the
+    /// information from this error.
+    ///
+    /// Returns the [`Error::raw_code`] value of this error, which is often
+    /// needed from a C callback.
+    pub(crate) unsafe fn raw_set_git_error(&self) -> raw::git_error_code {
+        let s = CString::new(self.message()).unwrap();
+        raw::git_error_set_str(self.class() as c_int, s.as_ptr());
+        self.raw_code()
+    }
 }
 
 impl error::Error for Error {}

src/remote_callbacks.rs

@@ -1,5 +1,5 @@
 use libc::{c_char, c_int, c_uint, c_void, size_t};
-use std::ffi::{CStr, CString};
+use std::ffi::CStr;
 use std::mem;
 use std::ptr;
 use std::slice;
@@ -330,11 +330,7 @@ extern "C" fn credentials_cb(
 
             let cred_type = CredentialType::from_bits_truncate(allowed_types as u32);
 
-            callback(url, username_from_url, cred_type).map_err(|e| {
-                let s = CString::new(e.to_string()).unwrap();
-                raw::git_error_set_str(e.class() as c_int, s.as_ptr());
-                e.raw_code() as c_int
-            })
+            callback(url, username_from_url, cred_type).map_err(|e| e.raw_set_git_error())
         });
         match ok {
             Some(Ok(cred)) => {
@@ -433,13 +429,7 @@ extern "C" fn certificate_check_cb(
     match ok {
         Some(Ok(CertificateCheckStatus::CertificateOk)) => 0,
         Some(Ok(CertificateCheckStatus::CertificatePassthrough)) => raw::GIT_PASSTHROUGH as c_int,
-        Some(Err(e)) => {
-            let s = CString::new(e.message()).unwrap();
-            unsafe {
-                raw::git_error_set_str(e.class() as c_int, s.as_ptr());
-            }
-            e.raw_code() as c_int
-        }
+        Some(Err(e)) => unsafe { e.raw_set_git_error() },
         None => {
             // Panic. The *should* get resumed by some future call to check().
             -1
@@ -466,7 +456,7 @@ extern "C" fn push_update_reference_cb(
         };
         match callback(refname, status) {
             Ok(()) => 0,
-            Err(e) => e.raw_code(),
+            Err(e) => e.raw_set_git_error(),
         }
     })
     .unwrap_or(-1)
@@ -529,7 +519,7 @@ extern "C" fn push_negotiation_cb(
         let updates = slice::from_raw_parts(updates as *mut PushUpdate<'_>, len);
         match callback(updates) {
             Ok(()) => 0,
-            Err(e) => e.raw_code(),
+            Err(e) => e.raw_set_git_error(),
         }
     })
     .unwrap_or(-1)

src/transport.rs

@@ -259,10 +259,7 @@ extern "C" fn subtransport_action(
         if generate_stream {
             let obj = match transport.obj.action(url, action) {
                 Ok(s) => s,
-                Err(e) => {
-                    set_err(&e);
-                    return e.raw_code() as c_int;
-                }
+                Err(e) => return e.raw_set_git_error(),
             };
             *stream = mem::transmute(Box::new(RawSmartSubtransportStream {
                 raw: raw::git_smart_subtransport_stream {
@@ -363,11 +360,6 @@ unsafe fn set_err_io(e: &io::Error) {
     raw::git_error_set_str(raw::GIT_ERROR_NET as c_int, s.as_ptr());
 }
 
-unsafe fn set_err(e: &Error) {
-    let s = CString::new(e.message()).unwrap();
-    raw::git_error_set_str(e.raw_class() as c_int, s.as_ptr());
-}
-
 // callback used by smart transports to free a `SmartSubtransportStream`
 // object.
 extern "C" fn stream_free(stream: *mut raw::git_smart_subtransport_stream) {