From b0cc44aabbdc09a31fd9cf20d438300d492975de Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Sat, 21 Apr 2018 12:54:48 +0200 Subject: [PATCH] ParityShell::open `Return result` (#8377) * start * add error handling for winapi * fix typo * fix warnings and windows errors * formatting * Address review comments --- parity/main.rs | 2 +- parity/run.rs | 5 ++--- parity/url.rs | 52 +++++++++++++++++++++++++++++++++++++++++--------- 3 files changed, 46 insertions(+), 13 deletions(-) diff --git a/parity/main.rs b/parity/main.rs index 705cf777e..6774a8386 100644 --- a/parity/main.rs +++ b/parity/main.rs @@ -231,7 +231,7 @@ fn take_spec_name_override() -> Option { #[cfg(windows)] fn global_cleanup() { // We need to cleanup all sockets before spawning another Parity process. This makes shure everything is cleaned up. - // The loop is required because of internal refernce counter for winsock dll. We don't know how many crates we use do + // The loop is required because of internal reference counter for winsock dll. We don't know how many crates we use do // initialize it. There's at least 2 now. for _ in 0.. 10 { unsafe { ::winapi::um::winsock2::WSACleanup(); } diff --git a/parity/run.rs b/parity/run.rs index 6ccd4caf5..f07f67caa 100644 --- a/parity/run.rs +++ b/parity/run.rs @@ -145,7 +145,7 @@ pub fn open_ui(ws_conf: &rpc::WsConfiguration, ui_conf: &rpc::UiConfiguration, l let token = signer::generate_token_and_url(ws_conf, ui_conf, logger_config)?; // Open a browser - url::open(&token.url); + url::open(&token.url).map_err(|e| format!("{}", e))?; // Print a message println!("{}", token.message); Ok(()) @@ -157,10 +157,9 @@ pub fn open_dapp(dapps_conf: &dapps::Configuration, rpc_conf: &rpc::HttpConfigur } let url = format!("http://{}:{}/{}/", rpc_conf.interface, rpc_conf.port, dapp); - url::open(&url); + url::open(&url).map_err(|e| format!("{}", e))?; Ok(()) } - // node info fetcher for the local store. struct FullNodeInfo { miner: Option>, // TODO: only TXQ needed, just use that after decoupling. diff --git a/parity/url.rs b/parity/url.rs index 4189ae241..41c4e5458 100644 --- a/parity/url.rs +++ b/parity/url.rs @@ -16,33 +16,67 @@ //! Cross-platform open url in default browser +use std; +use std::os::raw::c_int; + +#[allow(unused)] +pub enum Error { + ProcessError(std::io::Error), + WindowsShellExecute(c_int), +} + +impl From for Error { + fn from(err: std::io::Error) -> Self { + Error::ProcessError(err) + } +} + +impl std::fmt::Display for Error { + fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> { + match *self { + Error::ProcessError(ref e) => write!(f, "{}", e), + Error::WindowsShellExecute(e) => write!(f, "WindowsShellExecute error: {}", e), + } + } +} + #[cfg(windows)] -pub fn open(url: &str) { +pub fn open(url: &str) -> Result<(), Error> { use std::ffi::CString; use std::ptr; use winapi::um::shellapi::ShellExecuteA; use winapi::um::winuser::SW_SHOWNORMAL as Normal; - unsafe { + const WINDOWS_SHELL_EXECUTE_SUCCESS: c_int = 32; + + let h_instance = unsafe { ShellExecuteA(ptr::null_mut(), CString::new("open").unwrap().as_ptr(), CString::new(url.to_owned().replace("\n", "%0A")).unwrap().as_ptr(), ptr::null(), ptr::null(), - Normal); + Normal) as c_int + }; + + // https://msdn.microsoft.com/en-us/library/windows/desktop/bb762153(v=vs.85).aspx + // `ShellExecute` returns a value greater than 32 on success + if h_instance > WINDOWS_SHELL_EXECUTE_SUCCESS { + Ok(()) + } else { + Err(Error::WindowsShellExecute(h_instance)) } } #[cfg(any(target_os="macos", target_os="freebsd"))] -pub fn open(url: &str) { - use std; - let _ = std::process::Command::new("open").arg(url).spawn(); +pub fn open(url: &str) -> Result<(), Error> { + let _ = std::process::Command::new("open").arg(url).spawn()?; + Ok(()) } #[cfg(target_os="linux")] -pub fn open(url: &str) { - use std; - let _ = std::process::Command::new("xdg-open").arg(url).spawn(); +pub fn open(url: &str) -> Result<(), Error> { + let _ = std::process::Command::new("xdg-open").arg(url).spawn()?; + Ok(()) } #[cfg(target_os="android")]