From 18bfc89a4c5badd3869b218caa83e0a422d8e4b4 Mon Sep 17 00:00:00 2001
From: Eric Huss <eric@huss.org>
Date: Sat, 13 Apr 2024 15:21:45 -0700
Subject: [PATCH] Consistently use the error message from a callback error.

---
 src/error.rs            | 13 ++++++++++++-
 src/remote_callbacks.rs | 20 +++++---------------
 src/transport.rs        | 10 +---------
 3 files changed, 18 insertions(+), 25 deletions(-)

diff --git a/src/error.rs b/src/error.rs
index 6f1c4d4c78..32f943c73f 100644
--- a/src/error.rs
+++ b/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 {}
diff --git a/src/remote_callbacks.rs b/src/remote_callbacks.rs
index 1169420bda..4a5f76d853 100644
--- a/src/remote_callbacks.rs
+++ b/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;
@@ -312,11 +312,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)) => {
@@ -415,13 +411,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
@@ -448,7 +438,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)
@@ -511,7 +501,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)
diff --git a/src/transport.rs b/src/transport.rs
index 74446d0caf..3a4660627f 100644
--- a/src/transport.rs
+++ b/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) {