Improve FFI error returning

3 files changed, 47 insertions, 18 deletions

diff --git a/talpid-core/src/firewall/windows/route.rs b/talpid-core/src/firewall/windows/route.rs
index c66dfbfabd..6ecf336956 100644
--- a/talpid-core/src/firewall/windows/route.rs
+++ b/talpid-core/src/firewall/windows/route.rs
@@ -9,35 +9,55 @@ error_chain!{
         MetricApplication{
             description("Failed to set the metrics for a network interface")
         }
+        InvalidInterfaceAlias{
+            description("Supplied interface alias is invalid")
+        }
     }
 }
 
-// Returns true if metrics were changed, false otherwise
+/// Returns true if metrics were changed, false otherwise
 pub fn ensure_top_metric_for_interface(interface_alias: &str) -> Result<bool> {
     let interface_alias_ws =
-        WideCString::new(interface_alias.encode_utf16().collect::<Vec<_>>()).unwrap();
-    match unsafe {
+        WideCString::from_str(interface_alias).chain_err(|| ErrorKind::InvalidInterfaceAlias)?;
+    unsafe {
         WinRoute_EnsureTopMetric(
             interface_alias_ws.as_wide_c_str().as_ptr(),
             Some(ffi::error_sink),
             ptr::null_mut(),
-        )
-    } {
-        0 => Ok(false),
-        1 => Ok(true),
-        -1 => Err(Error::from(ErrorKind::MetricApplication)),
-        _ => {
-            error!("Unexpected return code from WinRoute_EnsureTopMetric");
-            Err(Error::from(ErrorKind::MetricApplication))
+        ).into()
+    }
+}
+
+// Allowing dead code here as this type should only ever be constructed by an
+// FFI function.
+#[allow(dead_code)]
+#[repr(u32)]
+enum MetricResult {
+    MetricsUnchanged = 0u32,
+    MetricsChanged = 1u32,
+    Failure = 2u32,
+    UnexpectedValue,
+}
+
+impl Into<Result<bool>> for MetricResult {
+    fn into(self) -> Result<bool> {
+        match self {
+            MetricResult::MetricsUnchanged => Ok(false),
+            MetricResult::MetricsChanged => Ok(true),
+            MetricResult::Failure => Err(Error::from(ErrorKind::MetricApplication)),
+            MetricResult::UnexpectedValue => {
+                error!("Unexpected return code from WinRoute_EnsureTopMetric");
+                Err(Error::from(ErrorKind::MetricApplication))
+            }
         }
     }
 }
 
 extern "system" {
     #[link_name(WinRoute_EnsureTopMetric)]
-    pub fn WinRoute_EnsureTopMetric(
+    fn WinRoute_EnsureTopMetric(
         tunnel_interface_alias: *const libc::wchar_t,
         sink: Option<ffi::ErrorSink>,
         sink_context: *mut libc::c_void,
-    ) -> i32;
+    ) -> MetricResult;
 }
diff --git a/windows/winroute/src/winroute/winroute.cpp b/windows/winroute/src/winroute/winroute.cpp
index 3c0b8ce2dd..fe5d82d3c3 100644
--- a/windows/winroute/src/winroute/winroute.cpp
+++ b/windows/winroute/src/winroute/winroute.cpp
@@ -4,9 +4,10 @@
 #include <cstdint>

 #include <stdexcept>

 

+

 extern "C"

 WINROUTE_LINKAGE

-int32_t

+WINROUTE_STATUS

 WINROUTE_API

 WinRoute_EnsureTopMetric(

 	const wchar_t *deviceAlias,

@@ -17,7 +18,7 @@ WinRoute_EnsureTopMetric(
 	{

 		NetworkInterfaces interfaces;

 		bool metrics_set = interfaces.SetTopMetricForInterfaceByAlias(deviceAlias);

-		return metrics_set ? 1 : 0;

+		return metrics_set ? WINROUTE_STATUS::METRIC_SET : WINROUTE_STATUS::METRIC_NO_CHANGE;

 	}

 	catch (std::exception &err) 

 	{

@@ -25,11 +26,12 @@ WinRoute_EnsureTopMetric(
 		{

 			errorSink(err.what(), errorSinkContext);

 		}

-		return -1;

+		return WINROUTE_STATUS::FAILURE;

+

 	}

 	catch (...)

 	{

-		return -1;

+		return WINROUTE_STATUS::FAILURE;

 	}

 };

 

diff --git a/windows/winroute/src/winroute/winroute.h b/windows/winroute/src/winroute/winroute.h
index 316a024d79..593108936e 100644
--- a/windows/winroute/src/winroute/winroute.h
+++ b/windows/winroute/src/winroute/winroute.h
@@ -11,10 +11,17 @@
 
 typedef void (WINROUTE_API *WinRouteErrorSink)(const char *errorMessage, void *context);
 
+enum class WINROUTE_STATUS : uint32_t
+{
+	METRIC_NO_CHANGE = 0,
+	METRIC_SET = 1,
+	FAILURE = 2,
+};
+
 
 extern "C"
 WINROUTE_LINKAGE
-int32_t
+WINROUTE_STATUS
 WINROUTE_API
 WinRoute_EnsureTopMetric(
 		const wchar_t *deviceAlias,