rust-chg: leverage impl trait at argument position

Differential Revision: https://phab.mercurial-scm.org/D8401

rust/chg/src/clientext.rs

@@ -1,136 +1,128 b''
 // Copyright 2018 Yuya Nishihara <[e-mail]>
 //
 // This software may be used and distributed according to the terms of the
 // GNU General Public License version 2 or any later version.
 
 //! cHg extensions to command server client.
 
 use bytes::{BufMut, Bytes, BytesMut};
 use std::ffi::OsStr;
 use std::io;
 use std::mem;
 use std::os::unix::ffi::OsStrExt;
 use std::os::unix::io::AsRawFd;
 use std::path::Path;
 use tokio_hglib::protocol::{OneShotQuery, OneShotRequest};
 use tokio_hglib::{Client, Connection};
 
 use crate::attachio::AttachIo;
 use crate::message::{self, Instruction};
 use crate::runcommand::ChgRunCommand;
 use crate::uihandler::SystemHandler;
 
 pub trait ChgClientExt<C>
 where
     C: Connection + AsRawFd,
 {
     /// Attaches the client file descriptors to the server.
     fn attach_io<I, O, E>(self, stdin: I, stdout: O, stderr: E) -> AttachIo<C, I, O, E>
     where
         I: AsRawFd,
         O: AsRawFd,
         E: AsRawFd;
 
     /// Changes the working directory of the server.
-    fn set_current_dir<P>(self, dir: P) -> OneShotRequest<C>
-    where
-        P: AsRef<Path>;
+    fn set_current_dir(self, dir: impl AsRef<Path>) -> OneShotRequest<C>;
 
     /// Updates the environment variables of the server.
-    fn set_env_vars_os<I, P>(self, vars: I) -> OneShotRequest<C>
-    where
-        I: IntoIterator<Item = (P, P)>,
-        P: AsRef<OsStr>;
+    fn set_env_vars_os(
+        self,
+        vars: impl IntoIterator<Item = (impl AsRef<OsStr>, impl AsRef<OsStr>)>,
+    ) -> OneShotRequest<C>;
 
     /// Changes the process title of the server.
-    fn set_process_name<P>(self, name: P) -> OneShotRequest<C>
-    where
-        P: AsRef<OsStr>;
+    fn set_process_name(self, name: impl AsRef<OsStr>) -> OneShotRequest<C>;
 
     /// Changes the umask of the server process.
     fn set_umask(self, mask: u32) -> OneShotRequest<C>;
 
     /// Runs the specified Mercurial command with cHg extension.
-    fn run_command_chg<I, P, H>(self, handler: H, args: I) -> ChgRunCommand<C, H>
+    fn run_command_chg<H>(
+        self,
+        handler: H,
+        args: impl IntoIterator<Item = impl AsRef<OsStr>>,
+    ) -> ChgRunCommand<C, H>
     where
-        I: IntoIterator<Item = P>,
-        P: AsRef<OsStr>,
         H: SystemHandler;
 
     /// Validates if the server can run Mercurial commands with the expected
     /// configuration.
     ///
     /// The `args` should contain early command arguments such as `--config`
     /// and `-R`.
     ///
     /// Client-side environment must be sent prior to this request, by
     /// `set_current_dir()` and `set_env_vars_os()`.
-    fn validate<I, P>(self, args: I) -> OneShotQuery<C, fn(Bytes) -> io::Result<Vec<Instruction>>>
-    where
-        I: IntoIterator<Item = P>,
-        P: AsRef<OsStr>;
+    fn validate(
+        self,
+        args: impl IntoIterator<Item = impl AsRef<OsStr>>,
+    ) -> OneShotQuery<C, fn(Bytes) -> io::Result<Vec<Instruction>>>;
 }
 
 impl<C> ChgClientExt<C> for Client<C>
 where
     C: Connection + AsRawFd,
 {
     fn attach_io<I, O, E>(self, stdin: I, stdout: O, stderr: E) -> AttachIo<C, I, O, E>
     where
         I: AsRawFd,
         O: AsRawFd,
         E: AsRawFd,
     {
         AttachIo::with_client(self, stdin, stdout, Some(stderr))
     }
 
-    fn set_current_dir<P>(self, dir: P) -> OneShotRequest<C>
-    where
-        P: AsRef<Path>,
-    {
+    fn set_current_dir(self, dir: impl AsRef<Path>) -> OneShotRequest<C> {
         OneShotRequest::start_with_args(self, b"chdir", dir.as_ref().as_os_str().as_bytes())
     }
 
-    fn set_env_vars_os<I, P>(self, vars: I) -> OneShotRequest<C>
-    where
-        I: IntoIterator<Item = (P, P)>,
-        P: AsRef<OsStr>,
-    {
+    fn set_env_vars_os(
+        self,
+        vars: impl IntoIterator<Item = (impl AsRef<OsStr>, impl AsRef<OsStr>)>,
+    ) -> OneShotRequest<C> {
         OneShotRequest::start_with_args(self, b"setenv", message::pack_env_vars_os(vars))
     }
 
-    fn set_process_name<P>(self, name: P) -> OneShotRequest<C>
-    where
-        P: AsRef<OsStr>,
-    {
+    fn set_process_name(self, name: impl AsRef<OsStr>) -> OneShotRequest<C> {
         OneShotRequest::start_with_args(self, b"setprocname", name.as_ref().as_bytes())
     }
 
     fn set_umask(self, mask: u32) -> OneShotRequest<C> {
         let mut args = BytesMut::with_capacity(mem::size_of_val(&mask));
         args.put_u32_be(mask);
         OneShotRequest::start_with_args(self, b"setumask2", args)
     }
 
-    fn run_command_chg<I, P, H>(self, handler: H, args: I) -> ChgRunCommand<C, H>
+    fn run_command_chg<H>(
+        self,
+        handler: H,
+        args: impl IntoIterator<Item = impl AsRef<OsStr>>,
+    ) -> ChgRunCommand<C, H>
     where
-        I: IntoIterator<Item = P>,
-        P: AsRef<OsStr>,
         H: SystemHandler,
     {
         ChgRunCommand::with_client(self, handler, message::pack_args_os(args))
     }
 
-    fn validate<I, P>(self, args: I) -> OneShotQuery<C, fn(Bytes) -> io::Result<Vec<Instruction>>>
-    where
-        I: IntoIterator<Item = P>,
-        P: AsRef<OsStr>,
-    {
+    fn validate(
+        self,
+        args: impl IntoIterator<Item = impl AsRef<OsStr>>,
+    ) -> OneShotQuery<C, fn(Bytes) -> io::Result<Vec<Instruction>>> {
         OneShotQuery::start_with_args(
             self,
             b"validate",
             message::pack_args_os(args),
             message::parse_instructions,
         )
     }
 }

rust/chg/src/locator.rs

@@ -1,501 +1,490 b''
 // Copyright 2011, 2018 Yuya Nishihara <[e-mail]>
 //
 // This software may be used and distributed according to the terms of the
 // GNU General Public License version 2 or any later version.
 
 //! Utility for locating command-server process.
 
 use futures::future::{self, Either, Loop};
 use log::debug;
 use std::env;
 use std::ffi::{OsStr, OsString};
 use std::fs::{self, DirBuilder};
 use std::io;
 use std::os::unix::ffi::{OsStrExt, OsStringExt};
 use std::os::unix::fs::{DirBuilderExt, MetadataExt};
 use std::path::{Path, PathBuf};
 use std::process::{self, Command};
 use std::time::Duration;
 use tokio::prelude::*;
 use tokio_hglib::UnixClient;
 use tokio_process::{Child, CommandExt};
 use tokio_timer;
 
 use crate::clientext::ChgClientExt;
 use crate::message::{Instruction, ServerSpec};
 use crate::procutil;
 
 const REQUIRED_SERVER_CAPABILITIES: &[&str] = &[
     "attachio",
     "chdir",
     "runcommand",
     "setenv",
     "setumask2",
     "validate",
 ];
 
 /// Helper to connect to and spawn a server process.
 #[derive(Clone, Debug)]
 pub struct Locator {
     hg_command: OsString,
     hg_early_args: Vec<OsString>,
     current_dir: PathBuf,
     env_vars: Vec<(OsString, OsString)>,
     process_id: u32,
     base_sock_path: PathBuf,
     redirect_sock_path: Option<PathBuf>,
     timeout: Duration,
 }
 
 impl Locator {
     /// Creates locator capturing the current process environment.
     ///
     /// If no `$CHGSOCKNAME` is specified, the socket directory will be
     /// created as necessary.
     pub fn prepare_from_env() -> io::Result<Locator> {
         Ok(Locator {
             hg_command: default_hg_command(),
             hg_early_args: Vec::new(),
             current_dir: env::current_dir()?,
             env_vars: env::vars_os().collect(),
             process_id: process::id(),
             base_sock_path: prepare_server_socket_path()?,
             redirect_sock_path: None,
             timeout: default_timeout(),
         })
     }
 
     /// Temporary socket path for this client process.
     fn temp_sock_path(&self) -> PathBuf {
         let src = self.base_sock_path.as_os_str().as_bytes();
         let mut buf = Vec::with_capacity(src.len() + 6); // "{src}.{pid}".len()
         buf.extend_from_slice(src);
         buf.extend_from_slice(format!(".{}", self.process_id).as_bytes());
         OsString::from_vec(buf).into()
     }
 
     /// Specifies the arguments to be passed to the server at start.
-    pub fn set_early_args<I, P>(&mut self, args: I)
-    where
-        I: IntoIterator<Item = P>,
-        P: AsRef<OsStr>,
-    {
+    pub fn set_early_args(&mut self, args: impl IntoIterator<Item = impl AsRef<OsStr>>) {
         self.hg_early_args = args.into_iter().map(|a| a.as_ref().to_owned()).collect();
     }
 
     /// Connects to the server.
     ///
     /// The server process will be spawned if not running.
     pub fn connect(self) -> impl Future<Item = (Self, UnixClient), Error = io::Error> {
         future::loop_fn((self, 0), |(loc, cnt)| {
             if cnt < 10 {
                 let fut = loc
                     .try_connect()
                     .and_then(|(loc, client)| {
                         client
                             .validate(&loc.hg_early_args)
                             .map(|(client, instructions)| (loc, client, instructions))
                     })
                     .and_then(move |(loc, client, instructions)| {
                         loc.run_instructions(client, instructions, cnt)
                     });
                 Either::A(fut)
             } else {
                 let msg = format!(
                     concat!(
                         "too many redirections.\n",
                         "Please make sure {:?} is not a wrapper which ",
                         "changes sensitive environment variables ",
                         "before executing hg. If you have to use a ",
                         "wrapper, wrap chg instead of hg.",
                     ),
                     loc.hg_command
                 );
                 Either::B(future::err(io::Error::new(io::ErrorKind::Other, msg)))
             }
         })
     }
 
     /// Runs instructions received from the server.
     fn run_instructions(
         mut self,
         client: UnixClient,
         instructions: Vec<Instruction>,
         cnt: usize,
     ) -> io::Result<Loop<(Self, UnixClient), (Self, usize)>> {
         let mut reconnect = false;
         for inst in instructions {
             debug!("instruction: {:?}", inst);
             match inst {
                 Instruction::Exit(_) => {
                     // Just returns the current connection to run the
                     // unparsable command and report the error
                     return Ok(Loop::Break((self, client)));
                 }
                 Instruction::Reconnect => {
                     reconnect = true;
                 }
                 Instruction::Redirect(path) => {
                     if path.parent() != self.base_sock_path.parent() {
                         let msg = format!(
                             "insecure redirect instruction from server: {}",
                             path.display()
                         );
                         return Err(io::Error::new(io::ErrorKind::InvalidData, msg));
                     }
                     self.redirect_sock_path = Some(path);
                     reconnect = true;
                 }
                 Instruction::Unlink(path) => {
                     if path.parent() != self.base_sock_path.parent() {
                         let msg = format!(
                             "insecure unlink instruction from server: {}",
                             path.display()
                         );
                         return Err(io::Error::new(io::ErrorKind::InvalidData, msg));
                     }
                     fs::remove_file(path).unwrap_or(()); // may race
                 }
             }
         }
 
         if reconnect {
             Ok(Loop::Continue((self, cnt + 1)))
         } else {
             Ok(Loop::Break((self, client)))
         }
     }
 
     /// Tries to connect to the existing server, or spawns new if not running.
     fn try_connect(self) -> impl Future<Item = (Self, UnixClient), Error = io::Error> {
         let sock_path = self
             .redirect_sock_path
             .as_ref()
             .unwrap_or(&self.base_sock_path)
             .clone();
         debug!("try connect to {}", sock_path.display());
         UnixClient::connect(sock_path)
             .then(|res| {
                 match res {
                     Ok(client) => Either::A(future::ok((self, client))),
                     Err(_) => {
                         // Prevent us from being re-connected to the outdated
                         // master server: We were told by the server to redirect
                         // to redirect_sock_path, which didn't work. We do not
                         // want to connect to the same master server again
                         // because it would probably tell us the same thing.
                         if self.redirect_sock_path.is_some() {
                             fs::remove_file(&self.base_sock_path).unwrap_or(());
                             // may race
                         }
                         Either::B(self.spawn_connect())
                     }
                 }
             })
             .and_then(|(loc, client)| {
                 check_server_capabilities(client.server_spec())?;
                 Ok((loc, client))
             })
             .and_then(|(loc, client)| {
                 // It's purely optional, and the server might not support this command.
                 if client.server_spec().capabilities.contains("setprocname") {
                     let fut = client
                         .set_process_name(format!("chg[worker/{}]", loc.process_id))
                         .map(|client| (loc, client));
                     Either::A(fut)
                 } else {
                     Either::B(future::ok((loc, client)))
                 }
             })
             .and_then(|(loc, client)| {
                 client
                     .set_current_dir(&loc.current_dir)
                     .map(|client| (loc, client))
             })
             .and_then(|(loc, client)| {
                 client
                     .set_env_vars_os(loc.env_vars.iter().cloned())
                     .map(|client| (loc, client))
             })
     }
 
     /// Spawns new server process and connects to it.
     ///
     /// The server will be spawned at the current working directory, then
     /// chdir to "/", so that the server will load configs from the target
     /// repository.
     fn spawn_connect(self) -> impl Future<Item = (Self, UnixClient), Error = io::Error> {
         let sock_path = self.temp_sock_path();
         debug!("start cmdserver at {}", sock_path.display());
         Command::new(&self.hg_command)
             .arg("serve")
             .arg("--cmdserver")
             .arg("chgunix")
             .arg("--address")
             .arg(&sock_path)
             .arg("--daemon-postexec")
             .arg("chdir:/")
             .args(&self.hg_early_args)
             .current_dir(&self.current_dir)
             .env_clear()
             .envs(self.env_vars.iter().cloned())
             .env("CHGINTERNALMARK", "")
             .spawn_async()
             .into_future()
             .and_then(|server| self.connect_spawned(server, sock_path))
             .and_then(|(loc, client, sock_path)| {
                 debug!(
                     "rename {} to {}",
                     sock_path.display(),
                     loc.base_sock_path.display()
                 );
                 fs::rename(&sock_path, &loc.base_sock_path)?;
                 Ok((loc, client))
             })
     }
 
     /// Tries to connect to the just spawned server repeatedly until timeout
     /// exceeded.
     fn connect_spawned(
         self,
         server: Child,
         sock_path: PathBuf,
     ) -> impl Future<Item = (Self, UnixClient, PathBuf), Error = io::Error> {
         debug!("try connect to {} repeatedly", sock_path.display());
         let connect = future::loop_fn(sock_path, |sock_path| {
             UnixClient::connect(sock_path.clone()).then(|res| {
                 match res {
                     Ok(client) => Either::A(future::ok(Loop::Break((client, sock_path)))),
                     Err(_) => {
                         // try again with slight delay
                         let fut = tokio_timer::sleep(Duration::from_millis(10))
                             .map(|()| Loop::Continue(sock_path))
                             .map_err(|err| io::Error::new(io::ErrorKind::Other, err));
                         Either::B(fut)
                     }
                 }
             })
         });
 
         // waits for either connection established or server failed to start
         connect
             .select2(server)
             .map_err(|res| res.split().0)
             .timeout(self.timeout)
             .map_err(|err| {
                 err.into_inner().unwrap_or_else(|| {
                     io::Error::new(
                         io::ErrorKind::TimedOut,
                         "timed out while connecting to server",
                     )
                 })
             })
             .and_then(|res| {
                 match res {
                     Either::A(((client, sock_path), server)) => {
                         server.forget(); // continue to run in background
                         Ok((self, client, sock_path))
                     }
                     Either::B((st, _)) => Err(io::Error::new(
                         io::ErrorKind::Other,
                         format!("server exited too early: {}", st),
                     )),
                 }
             })
     }
 }
 
 /// Determines the server socket to connect to.
 ///
 /// If no `$CHGSOCKNAME` is specified, the socket directory will be created
 /// as necessary.
 fn prepare_server_socket_path() -> io::Result<PathBuf> {
     if let Some(s) = env::var_os("CHGSOCKNAME") {
         Ok(PathBuf::from(s))
     } else {
         let mut path = default_server_socket_dir();
         create_secure_dir(&path)?;
         path.push("server");
         Ok(path)
     }
 }
 
 /// Determines the default server socket path as follows.
 ///
 /// 1. `$XDG_RUNTIME_DIR/chg`
 /// 2. `$TMPDIR/chg$UID`
 /// 3. `/tmp/chg$UID`
 pub fn default_server_socket_dir() -> PathBuf {
     // XDG_RUNTIME_DIR should be ignored if it has an insufficient permission.
     // https://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html
     if let Some(Ok(s)) = env::var_os("XDG_RUNTIME_DIR").map(check_secure_dir) {
         let mut path = PathBuf::from(s);
         path.push("chg");
         path
     } else {
         let mut path = env::temp_dir();
         path.push(format!("chg{}", procutil::get_effective_uid()));
         path
     }
 }
 
 /// Determines the default hg command.
 pub fn default_hg_command() -> OsString {
     // TODO: maybe allow embedding the path at compile time (or load from hgrc)
     env::var_os("CHGHG")
         .or(env::var_os("HG"))
         .unwrap_or(OsStr::new("hg").to_owned())
 }
 
 fn default_timeout() -> Duration {
     let secs = env::var("CHGTIMEOUT")
         .ok()
         .and_then(|s| s.parse().ok())
         .unwrap_or(60);
     Duration::from_secs(secs)
 }
 
 /// Creates a directory which the other users cannot access to.
 ///
 /// If the directory already exists, tests its permission.
-fn create_secure_dir<P>(path: P) -> io::Result<()>
-where
-    P: AsRef<Path>,
-{
+fn create_secure_dir(path: impl AsRef<Path>) -> io::Result<()> {
     DirBuilder::new()
         .mode(0o700)
         .create(path.as_ref())
         .or_else(|err| {
             if err.kind() == io::ErrorKind::AlreadyExists {
                 check_secure_dir(path).map(|_| ())
             } else {
                 Err(err)
             }
         })
 }
 
 fn check_secure_dir<P>(path: P) -> io::Result<P>
 where
     P: AsRef<Path>,
 {
     let a = fs::symlink_metadata(path.as_ref())?;
     if a.is_dir() && a.uid() == procutil::get_effective_uid() && (a.mode() & 0o777) == 0o700 {
         Ok(path)
     } else {
         Err(io::Error::new(io::ErrorKind::Other, "insecure directory"))
     }
 }
 
 fn check_server_capabilities(spec: &ServerSpec) -> io::Result<()> {
     let unsupported: Vec<_> = REQUIRED_SERVER_CAPABILITIES
         .iter()
         .cloned()
         .filter(|&s| !spec.capabilities.contains(s))
         .collect();
     if unsupported.is_empty() {
         Ok(())
     } else {
         let msg = format!(
             "insufficient server capabilities: {}",
             unsupported.join(", ")
         );
         Err(io::Error::new(io::ErrorKind::Other, msg))
     }
 }
 
 /// Collects arguments which need to be passed to the server at start.
-pub fn collect_early_args<I, P>(args: I) -> Vec<OsString>
-where
-    I: IntoIterator<Item = P>,
-    P: AsRef<OsStr>,
-{
+pub fn collect_early_args(args: impl IntoIterator<Item = impl AsRef<OsStr>>) -> Vec<OsString> {
     let mut args_iter = args.into_iter();
     let mut early_args = Vec::new();
     while let Some(arg) = args_iter.next() {
         let argb = arg.as_ref().as_bytes();
         if argb == b"--" {
             break;
         } else if argb.starts_with(b"--") {
             let mut split = argb[2..].splitn(2, |&c| c == b'=');
             match split.next().unwrap() {
                 b"traceback" => {
                     if split.next().is_none() {
                         early_args.push(arg.as_ref().to_owned());
                     }
                 }
                 b"config" | b"cwd" | b"repo" | b"repository" => {
                     if split.next().is_some() {
                         // --<flag>=<val>
                         early_args.push(arg.as_ref().to_owned());
                     } else {
                         // --<flag> <val>
                         args_iter.next().map(|val| {
                             early_args.push(arg.as_ref().to_owned());
                             early_args.push(val.as_ref().to_owned());
                         });
                     }
                 }
                 _ => {}
             }
         } else if argb.starts_with(b"-R") {
             if argb.len() > 2 {
                 // -R<val>
                 early_args.push(arg.as_ref().to_owned());
             } else {
                 // -R <val>
                 args_iter.next().map(|val| {
                     early_args.push(arg.as_ref().to_owned());
                     early_args.push(val.as_ref().to_owned());
                 });
             }
         }
     }
 
     early_args
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
 
     #[test]
     fn collect_early_args_some() {
         assert!(collect_early_args(&[] as &[&OsStr]).is_empty());
         assert!(collect_early_args(&["log"]).is_empty());
         assert_eq!(
             collect_early_args(&["log", "-Ra", "foo"]),
             os_string_vec_from(&[b"-Ra"])
         );
         assert_eq!(
             collect_early_args(&["log", "-R", "repo", "", "--traceback", "a"]),
             os_string_vec_from(&[b"-R", b"repo", b"--traceback"])
         );
         assert_eq!(
             collect_early_args(&["log", "--config", "diff.git=1", "-q"]),
             os_string_vec_from(&[b"--config", b"diff.git=1"])
         );
         assert_eq!(
             collect_early_args(&["--cwd=..", "--repository", "r", "log"]),
             os_string_vec_from(&[b"--cwd=..", b"--repository", b"r"])
         );
         assert_eq!(
             collect_early_args(&["log", "--repo=r", "--repos", "a"]),
             os_string_vec_from(&[b"--repo=r"])
         );
     }
 
     #[test]
     fn collect_early_args_orphaned() {
         assert!(collect_early_args(&["log", "-R"]).is_empty());
         assert!(collect_early_args(&["log", "--config"]).is_empty());
     }
 
     #[test]
     fn collect_early_args_unwanted_value() {
         assert!(collect_early_args(&["log", "--traceback="]).is_empty());
     }
 
     fn os_string_vec_from(v: &[&[u8]]) -> Vec<OsString> {
         v.iter().map(|s| OsStr::from_bytes(s).to_owned()).collect()
     }
 }

rust/chg/src/message.rs

@@ -1,317 +1,309 b''
 // Copyright 2018 Yuya Nishihara <[e-mail]>
 //
 // This software may be used and distributed according to the terms of the
 // GNU General Public License version 2 or any later version.
 
 //! Utility for parsing and building command-server messages.
 
 use bytes::{BufMut, Bytes, BytesMut};
 use std::error;
 use std::ffi::{OsStr, OsString};
 use std::io;
 use std::os::unix::ffi::OsStrExt;
 use std::path::PathBuf;
 
 pub use tokio_hglib::message::*; // re-exports
 
 /// Shell command type requested by the server.
 #[derive(Clone, Copy, Debug, Eq, PartialEq)]
 pub enum CommandType {
     /// Pager should be spawned.
     Pager,
     /// Shell command should be executed to send back the result code.
     System,
 }
 
 /// Shell command requested by the server.
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub struct CommandSpec {
     pub command: OsString,
     pub current_dir: OsString,
     pub envs: Vec<(OsString, OsString)>,
 }
 
 /// Parses "S" channel request into command type and spec.
 pub fn parse_command_spec(data: Bytes) -> io::Result<(CommandType, CommandSpec)> {
     let mut split = data.split(|&c| c == b'\0');
     let ctype = parse_command_type(split.next().ok_or(new_parse_error("missing type"))?)?;
     let command = split.next().ok_or(new_parse_error("missing command"))?;
     let current_dir = split.next().ok_or(new_parse_error("missing current dir"))?;
 
     let mut envs = Vec::new();
     for l in split {
         let mut s = l.splitn(2, |&c| c == b'=');
         let k = s.next().unwrap();
         let v = s.next().ok_or(new_parse_error("malformed env"))?;
         envs.push((
             OsStr::from_bytes(k).to_owned(),
             OsStr::from_bytes(v).to_owned(),
         ));
     }
 
     let spec = CommandSpec {
         command: OsStr::from_bytes(command).to_owned(),
         current_dir: OsStr::from_bytes(current_dir).to_owned(),
         envs: envs,
     };
     Ok((ctype, spec))
 }
 
 fn parse_command_type(value: &[u8]) -> io::Result<CommandType> {
     match value {
         b"pager" => Ok(CommandType::Pager),
         b"system" => Ok(CommandType::System),
         _ => Err(new_parse_error(format!(
             "unknown command type: {}",
             decode_latin1(value)
         ))),
     }
 }
 
 /// Client-side instruction requested by the server.
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub enum Instruction {
     Exit(i32),
     Reconnect,
     Redirect(PathBuf),
     Unlink(PathBuf),
 }
 
 /// Parses validation result into instructions.
 pub fn parse_instructions(data: Bytes) -> io::Result<Vec<Instruction>> {
     let mut instructions = Vec::new();
     for l in data.split(|&c| c == b'\0') {
         if l.is_empty() {
             continue;
         }
         let mut s = l.splitn(2, |&c| c == b' ');
         let inst = match (s.next().unwrap(), s.next()) {
             (b"exit", Some(arg)) => decode_latin1(arg)
                 .parse()
                 .map(Instruction::Exit)
                 .map_err(|_| new_parse_error(format!("invalid exit code: {:?}", arg)))?,
             (b"reconnect", None) => Instruction::Reconnect,
             (b"redirect", Some(arg)) => {
                 Instruction::Redirect(OsStr::from_bytes(arg).to_owned().into())
             }
             (b"unlink", Some(arg)) => Instruction::Unlink(OsStr::from_bytes(arg).to_owned().into()),
             _ => {
                 return Err(new_parse_error(format!("unknown command: {:?}", l)));
             }
         };
         instructions.push(inst);
     }
     Ok(instructions)
 }
 
 // allocate large buffer as environment variables can be quite long
 const INITIAL_PACKED_ENV_VARS_CAPACITY: usize = 4096;
 
 /// Packs environment variables of platform encoding into bytes.
 ///
 /// # Panics
 ///
 /// Panics if key or value contains `\0` character, or key contains '='
 /// character.
-pub fn pack_env_vars_os<I, P>(vars: I) -> Bytes
-where
-    I: IntoIterator<Item = (P, P)>,
-    P: AsRef<OsStr>,
-{
+pub fn pack_env_vars_os(
+    vars: impl IntoIterator<Item = (impl AsRef<OsStr>, impl AsRef<OsStr>)>,
+) -> Bytes {
     let mut vars_iter = vars.into_iter();
     if let Some((k, v)) = vars_iter.next() {
         let mut dst = BytesMut::with_capacity(INITIAL_PACKED_ENV_VARS_CAPACITY);
         pack_env_into(&mut dst, k.as_ref(), v.as_ref());
         for (k, v) in vars_iter {
             dst.reserve(1);
             dst.put_u8(b'\0');
             pack_env_into(&mut dst, k.as_ref(), v.as_ref());
         }
         dst.freeze()
     } else {
         Bytes::new()
     }
 }
 
 fn pack_env_into(dst: &mut BytesMut, k: &OsStr, v: &OsStr) {
     assert!(!k.as_bytes().contains(&0), "key shouldn't contain NUL");
     assert!(!k.as_bytes().contains(&b'='), "key shouldn't contain '='");
     assert!(!v.as_bytes().contains(&0), "value shouldn't contain NUL");
     dst.reserve(k.as_bytes().len() + 1 + v.as_bytes().len());
     dst.put_slice(k.as_bytes());
     dst.put_u8(b'=');
     dst.put_slice(v.as_bytes());
 }
 
-fn decode_latin1<S>(s: S) -> String
-where
-    S: AsRef<[u8]>,
-{
+fn decode_latin1(s: impl AsRef<[u8]>) -> String {
     s.as_ref().iter().map(|&c| c as char).collect()
 }
 
-fn new_parse_error<E>(error: E) -> io::Error
-where
-    E: Into<Box<dyn error::Error + Send + Sync>>,
-{
+fn new_parse_error(error: impl Into<Box<dyn error::Error + Send + Sync>>) -> io::Error {
     io::Error::new(io::ErrorKind::InvalidData, error)
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
     use std::os::unix::ffi::OsStringExt;
     use std::panic;
 
     #[test]
     fn parse_command_spec_good() {
         let src = [
             b"pager".as_ref(),
             b"less -FRX".as_ref(),
             b"/tmp".as_ref(),
             b"LANG=C".as_ref(),
             b"HGPLAIN=".as_ref(),
         ]
         .join(&0);
         let spec = CommandSpec {
             command: os_string_from(b"less -FRX"),
             current_dir: os_string_from(b"/tmp"),
             envs: vec![
                 (os_string_from(b"LANG"), os_string_from(b"C")),
                 (os_string_from(b"HGPLAIN"), os_string_from(b"")),
             ],
         };
         assert_eq!(
             parse_command_spec(Bytes::from(src)).unwrap(),
             (CommandType::Pager, spec)
         );
     }
 
     #[test]
     fn parse_command_spec_too_short() {
         assert!(parse_command_spec(Bytes::from_static(b"")).is_err());
         assert!(parse_command_spec(Bytes::from_static(b"pager")).is_err());
         assert!(parse_command_spec(Bytes::from_static(b"pager\0less")).is_err());
     }
 
     #[test]
     fn parse_command_spec_malformed_env() {
         assert!(parse_command_spec(Bytes::from_static(b"pager\0less\0/tmp\0HOME")).is_err());
     }
 
     #[test]
     fn parse_command_spec_unknown_type() {
         assert!(parse_command_spec(Bytes::from_static(b"paper\0less")).is_err());
     }
 
     #[test]
     fn parse_instructions_good() {
         let src = [
             b"exit 123".as_ref(),
             b"reconnect".as_ref(),
             b"redirect /whatever".as_ref(),
             b"unlink /someother".as_ref(),
         ]
         .join(&0);
         let insts = vec![
             Instruction::Exit(123),
             Instruction::Reconnect,
             Instruction::Redirect(path_buf_from(b"/whatever")),
             Instruction::Unlink(path_buf_from(b"/someother")),
         ];
         assert_eq!(parse_instructions(Bytes::from(src)).unwrap(), insts);
     }
 
     #[test]
     fn parse_instructions_empty() {
         assert_eq!(parse_instructions(Bytes::new()).unwrap(), vec![]);
         assert_eq!(
             parse_instructions(Bytes::from_static(b"\0")).unwrap(),
             vec![]
         );
     }
 
     #[test]
     fn parse_instructions_malformed_exit_code() {
         assert!(parse_instructions(Bytes::from_static(b"exit foo")).is_err());
     }
 
     #[test]
     fn parse_instructions_missing_argument() {
         assert!(parse_instructions(Bytes::from_static(b"exit")).is_err());
         assert!(parse_instructions(Bytes::from_static(b"redirect")).is_err());
         assert!(parse_instructions(Bytes::from_static(b"unlink")).is_err());
     }
 
     #[test]
     fn parse_instructions_unknown_command() {
         assert!(parse_instructions(Bytes::from_static(b"quit 0")).is_err());
     }
 
     #[test]
     fn pack_env_vars_os_good() {
         assert_eq!(
             pack_env_vars_os(vec![] as Vec<(OsString, OsString)>),
             Bytes::new()
         );
         assert_eq!(
             pack_env_vars_os(vec![os_string_pair_from(b"FOO", b"bar")]),
             Bytes::from_static(b"FOO=bar")
         );
         assert_eq!(
             pack_env_vars_os(vec![
                 os_string_pair_from(b"FOO", b""),
                 os_string_pair_from(b"BAR", b"baz")
             ]),
             Bytes::from_static(b"FOO=\0BAR=baz")
         );
     }
 
     #[test]
     fn pack_env_vars_os_large_key() {
         let mut buf = vec![b'A'; INITIAL_PACKED_ENV_VARS_CAPACITY];
         let envs = vec![os_string_pair_from(&buf, b"")];
         buf.push(b'=');
         assert_eq!(pack_env_vars_os(envs), Bytes::from(buf));
     }
 
     #[test]
     fn pack_env_vars_os_large_value() {
         let mut buf = vec![b'A', b'='];
         buf.resize(INITIAL_PACKED_ENV_VARS_CAPACITY + 1, b'a');
         let envs = vec![os_string_pair_from(&buf[..1], &buf[2..])];
         assert_eq!(pack_env_vars_os(envs), Bytes::from(buf));
     }
 
     #[test]
     fn pack_env_vars_os_nul_eq() {
         assert!(panic::catch_unwind(|| {
             pack_env_vars_os(vec![os_string_pair_from(b"\0", b"")])
         })
         .is_err());
         assert!(panic::catch_unwind(|| {
             pack_env_vars_os(vec![os_string_pair_from(b"FOO", b"\0bar")])
         })
         .is_err());
         assert!(panic::catch_unwind(|| {
             pack_env_vars_os(vec![os_string_pair_from(b"FO=", b"bar")])
         })
         .is_err());
         assert_eq!(
             pack_env_vars_os(vec![os_string_pair_from(b"FOO", b"=ba")]),
             Bytes::from_static(b"FOO==ba")
         );
     }
 
     fn os_string_from(s: &[u8]) -> OsString {
         OsString::from_vec(s.to_vec())
     }
 
     fn os_string_pair_from(k: &[u8], v: &[u8]) -> (OsString, OsString) {
         (os_string_from(k), os_string_from(v))
     }
 
     fn path_buf_from(s: &[u8]) -> PathBuf {
         os_string_from(s).into()
     }
 }