From 82be50327f3575b16a348b8dd4f4eb14ba0b8214 Mon Sep 17 00:00:00 2001 From: Alexis Mousset Date: Tue, 11 Nov 2014 00:55:10 +0100 Subject: [PATCH] Avoid unnecessary clone() --- src/client/mod.rs | 60 ++++++++++++++------------- src/client/server_info.rs | 8 ++-- src/command.rs | 18 ++++---- src/error.rs | 6 +-- src/response.rs | 8 ++-- src/transaction.rs | 86 +++++++++++++++++++-------------------- 6 files changed, 95 insertions(+), 91 deletions(-) diff --git a/src/client/mod.rs b/src/client/mod.rs index bd09e64..684f7b3 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -49,18 +49,26 @@ pub struct Client { } macro_rules! try_smtp ( - ($expr:expr $sp: ident) => ({ - match $expr { + ($err: expr $client: ident) => ({ + match $err { Ok(val) => val, - Err(err) => fail_with_err!(err $sp) + Err(err) => fail_with_err!(err $client) } }) ) macro_rules! fail_with_err ( - ($expr:expr $sp: ident) => ({ - $sp.close_on_error::(); - return Err(FromError::from_error($expr)) + ($err: expr $client: ident) => ({ + $client.close_on_error::(); + return Err(FromError::from_error($err)) + }) +) + +macro_rules! check_command_sequence ( + ($command: ident $client: ident) => ({ + if !$client.state.is_command_allowed(&$command) { + fail_with_err!(Response{code: 503, message: Some("Bad sequence of commands".to_string())} $client); + } }) ) @@ -110,7 +118,7 @@ impl Client { _ => {} } - debug!("Server {}", self.server_info.clone().unwrap()); + debug!("Server {}", self.server_info.as_ref().unwrap()); // Mail try_smtp!(self.mail::(from_address) self); @@ -132,7 +140,7 @@ impl Client { let sent = try_smtp!(self.message::(message) self); info!("to=<{}>, status=sent ({})", - to_addresses.connect(">, to=<"), sent.clone()); + to_addresses.connect(">, to=<"), sent); // Quit try_smtp!(self.quit::() self); @@ -160,47 +168,43 @@ impl Client { /// * If `message` is `None`, the given command will be formatted and sent to the server /// * If `message` is `Some(str)`, the `str` string will be sent to the server fn send_server(&mut self, command: Command, message: Option<&str>) -> SmtpResult { - if !self.state.is_command_allowed(command.clone()) { - fail_with_err!(Response{code: 503, message: Some("Bad sequence of commands".to_string())} self); - } + check_command_sequence!(command self); let result = try!(match message { - Some(message) => self.stream.clone().unwrap().send_and_get_response( + Some(message) => self.stream.as_mut().unwrap().send_and_get_response( message, MESSAGE_ENDING ), - None => self.stream.clone().unwrap().send_and_get_response( + None => self.stream.as_mut().unwrap().send_and_get_response( command.to_string().as_slice(), CRLF ) }); - let checked_result = try!(command.clone().test_success(result)); - - self.state = self.state.next_state(command.clone()).unwrap(); - + let checked_result = try!(command.test_success(result)); + self.state = self.state.next_state(&command).unwrap(); Ok(checked_result) } /// Connects to the configured server pub fn connect(&mut self) -> SmtpResult { let command = command::Connect; + check_command_sequence!(command self); - // connect should not be called when the client is already connected + // Connect should not be called when the client is already connected if !self.stream.is_none() { fail_with_err!("The connection is already established" self); } // Try to connect - self.stream = Some(try!(Connecter::connect(self.host.clone().as_slice(), self.port))); + self.stream = Some(try!(Connecter::connect(self.host.as_slice(), self.port))); // Log the connection info!("Connection established to {}[{}]:{}", - self.host, self.stream.clone().unwrap().peer_name().unwrap().ip, self.port); + self.host, self.stream.as_mut().unwrap().peer_name().unwrap().ip, self.port); - let result = try!(self.stream.clone().unwrap().get_reply()); + let result = try!(self.stream.as_mut().unwrap().get_reply()); let checked_result = try!(command.test_success(result)); - - self.state = self.state.next_state(command).unwrap(); + self.state = self.state.next_state(&command).unwrap(); Ok(checked_result) } @@ -212,7 +216,7 @@ impl Client { /// Closes the TCP stream pub fn close(&mut self) { // Close the TCP connection - drop(self.stream.clone().unwrap()); + drop(self.stream.as_mut().unwrap()); // Reset client state self.stream = None; self.state = TransactionState::new(); @@ -224,7 +228,7 @@ impl Client { let result = try!(self.send_command(command::Hello(my_hostname.to_string()))); self.server_info = Some( ServerInfo{ - name: get_first_word(result.message.clone().unwrap().as_slice()).to_string(), + name: get_first_word(result.message.as_ref().unwrap().as_slice()).to_string(), esmtp_features: None } ); @@ -236,9 +240,9 @@ impl Client { let result = try!(self.send_command(command::ExtendedHello(my_hostname.to_string()))); self.server_info = Some( ServerInfo{ - name: get_first_word(result.message.clone().unwrap().as_slice()).to_string(), + name: get_first_word(result.message.as_ref().unwrap().as_slice()).to_string(), esmtp_features: Extension::parse_esmtp_response( - result.message.clone().unwrap().as_slice() + result.message.as_ref().unwrap().as_slice() ) } ); @@ -293,7 +297,7 @@ impl Client { // Check message encoding if !server_info.supports_feature(extension::EightBitMime).is_some() { - if !message_content.clone().is_ascii() { + if !message_content.is_ascii() { fail_with_err!("Server does not accepts UTF-8 strings" self); } } diff --git a/src/client/server_info.rs b/src/client/server_info.rs index 09e961c..9c8d0eb 100644 --- a/src/client/server_info.rs +++ b/src/client/server_info.rs @@ -33,8 +33,8 @@ impl Show for ServerInfo { f.write( format!("{} with {}", self.name, - match self.esmtp_features.clone() { - Some(features) => features.to_string(), + match self.esmtp_features { + Some(ref features) => features.to_string(), None => "no supported features".to_string() } ).as_bytes() @@ -45,8 +45,8 @@ impl Show for ServerInfo { impl ServerInfo { /// Checks if the server supports an ESMTP feature pub fn supports_feature(&self, keyword: Extension) -> Option { - match self.esmtp_features.clone() { - Some(esmtp_features) => { + match self.esmtp_features { + Some(ref esmtp_features) => { for feature in esmtp_features.iter() { if keyword.same_extension_as(*feature) { return Some(*feature); diff --git a/src/command.rs b/src/command.rs index d8b7e27..052927f 100644 --- a/src/command.rs +++ b/src/command.rs @@ -60,21 +60,21 @@ impl Show for Command { f.write( match *self { Connect => "CONNECT".to_string(), StartTls => "STARTTLS".to_string(), - ExtendedHello(ref my_hostname) => format!("EHLO {}", my_hostname.clone()), - Hello(ref my_hostname) => format!("HELO {}", my_hostname.clone()), - Mail(ref from_address, None) => format!("MAIL FROM:<{}>", from_address.clone()), + ExtendedHello(ref my_hostname) => format!("EHLO {}", my_hostname), + Hello(ref my_hostname) => format!("HELO {}", my_hostname), + Mail(ref from_address, None) => format!("MAIL FROM:<{}>", from_address), Mail(ref from_address, Some(ref options)) => - format!("MAIL FROM:<{}> {}", from_address.clone(), options.connect(SP)), - Recipient(ref to_address, None) => format!("RCPT TO:<{}>", to_address.clone()), + format!("MAIL FROM:<{}> {}", from_address, options.connect(SP)), + Recipient(ref to_address, None) => format!("RCPT TO:<{}>", to_address), Recipient(ref to_address, Some(ref options)) => - format!("RCPT TO:<{}> {}", to_address.clone(), options.connect(SP)), + format!("RCPT TO:<{}> {}", to_address, options.connect(SP)), Data => "DATA".to_string(), Message => "MESSAGE".to_string(), Reset => "RSET".to_string(), - Verify(ref address) => format!("VRFY {}", address.clone()), - Expand(ref address) => format!("EXPN {}", address.clone()), + Verify(ref address) => format!("VRFY {}", address), + Expand(ref address) => format!("EXPN {}", address), Help(None) => "HELP".to_string(), - Help(Some(ref argument)) => format!("HELP {}", argument.clone()), + Help(Some(ref argument)) => format!("HELP {}", argument), Noop => "NOOP".to_string(), Quit => "QUIT".to_string(), }.as_bytes()) diff --git a/src/error.rs b/src/error.rs index d97b9f0..68a6b78 100644 --- a/src/error.rs +++ b/src/error.rs @@ -66,9 +66,9 @@ impl FromError<(ErrorKind, &'static str)> for SmtpError { impl FromError for SmtpError { fn from_error(response: Response) -> SmtpError { let kind = match response.code/100 { - 4 => TransientError(response.clone()), - 5 => PermanentError(response.clone()), - _ => UnknownError(response.clone().to_string()), + 4 => TransientError(response), + 5 => PermanentError(response), + _ => UnknownError(response.to_string()), }; let desc = match kind { TransientError(_) => "a permanent error occured during the SMTP transaction", diff --git a/src/response.rs b/src/response.rs index 6a8f9dc..cfabac5 100644 --- a/src/response.rs +++ b/src/response.rs @@ -47,10 +47,10 @@ impl FromStr for Response { } else if s.len() == 3 || (s.len() == 4 && s.slice(3,4) == " ") { match from_str::(s.slice_to(3)) { Some(code) => Some(Response{ - code: code, - message: None - }), - None => None + code: code, + message: None + }), + None => None } // If we have a code and a message diff --git a/src/transaction.rs b/src/transaction.rs index 592b161..f68a527 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -38,12 +38,12 @@ impl Show for TransactionState { fn fmt(&self, f: &mut Formatter) -> fmt::Result { f.write( match *self { - Unconnected => "Unconnected", - Connected => "Connected", - HelloSent => "HelloSent", - MailSent => "MailSent", + Unconnected => "Unconnected", + Connected => "Connected", + HelloSent => "HelloSent", + MailSent => "MailSent", RecipientSent => "RecipientSent", - DataSent => "DataSent" + DataSent => "DataSent" }.as_bytes() ) } @@ -56,27 +56,27 @@ impl TransactionState { } /// Tests if the given command is allowed in the current state - pub fn is_command_allowed(&self, command: Command) -> bool { + pub fn is_command_allowed(&self, command: &Command) -> bool { match (*self, command) { - (Unconnected, command::Connect) => true, + (Unconnected, &command::Connect) => true, (Unconnected, _) => false, // Only a message can follow the DATA command - (DataSent, command::Message) => true, + (DataSent, &command::Message) => true, (DataSent, _) => false, // Commands that can be issued everytime - (_, command::ExtendedHello(_)) => true, - (_, command::Hello(_)) => true, - (_, command::Reset) => true, - (_, command::Verify(_)) => true, - (_, command::Expand(_)) => true, - (_, command::Help(_)) => true, - (_, command::Noop) => true, - (_, command::Quit) => true, + (_, &command::ExtendedHello(_)) => true, + (_, &command::Hello(_)) => true, + (_, &command::Reset) => true, + (_, &command::Verify(_)) => true, + (_, &command::Expand(_)) => true, + (_, &command::Help(_)) => true, + (_, &command::Noop) => true, + (_, &command::Quit) => true, // Commands that require a particular state - (HelloSent, command::Mail(_, _)) => true, - (MailSent, command::Recipient(_, _)) => true, - (RecipientSent, command::Recipient(_, _)) => true, - (RecipientSent, command::Data) => true, + (HelloSent, &command::Mail(_, _)) => true, + (MailSent, &command::Recipient(_, _)) => true, + (RecipientSent, &command::Recipient(_, _)) => true, + (RecipientSent, &command::Data) => true, // Everything else (_, _) => false } @@ -85,27 +85,27 @@ impl TransactionState { /// Returns the state resulting of the given command /// /// A `None` return value means the comand is not allowed. - pub fn next_state(&mut self, command: Command) -> Option { + pub fn next_state(&mut self, command: &Command) -> Option { match (*self, command) { - (Unconnected, command::Connect) => Some(Connected), + (Unconnected, &command::Connect) => Some(Connected), (Unconnected, _) => None, - (DataSent, command::Message) => Some(HelloSent), + (DataSent, &command::Message) => Some(HelloSent), (DataSent, _) => None, // Commands that can be issued everytime - (_, command::ExtendedHello(_)) => Some(HelloSent), - (_, command::Hello(_)) => Some(HelloSent), - (Connected, command::Reset) => Some(Connected), - (_, command::Reset) => Some(HelloSent), - (state, command::Verify(_)) => Some(state), - (state, command::Expand(_)) => Some(state), - (state, command::Help(_)) => Some(state), - (state, command::Noop) => Some(state), - (_, command::Quit) => Some(Unconnected), + (_, &command::ExtendedHello(_)) => Some(HelloSent), + (_, &command::Hello(_)) => Some(HelloSent), + (Connected, &command::Reset) => Some(Connected), + (_, &command::Reset) => Some(HelloSent), + (state, &command::Verify(_)) => Some(state), + (state, &command::Expand(_)) => Some(state), + (state, &command::Help(_)) => Some(state), + (state, &command::Noop) => Some(state), + (_, &command::Quit) => Some(Unconnected), // Commands that require a particular state - (HelloSent, command::Mail(_, _)) => Some(MailSent), - (MailSent, command::Recipient(_, _)) => Some(RecipientSent), - (RecipientSent, command::Recipient(_, _)) => Some(RecipientSent), - (RecipientSent, command::Data) => Some(DataSent), + (HelloSent, &command::Mail(_, _)) => Some(MailSent), + (MailSent, &command::Recipient(_, _)) => Some(RecipientSent), + (RecipientSent, &command::Recipient(_, _)) => Some(RecipientSent), + (RecipientSent, &command::Data) => Some(DataSent), // Everything else (_, _) => None } @@ -124,17 +124,17 @@ mod test { #[test] fn test_is_command_allowed() { - assert!(!super::Unconnected.is_command_allowed(command::Noop)); - assert!(!super::DataSent.is_command_allowed(command::Noop)); - assert!(super::HelloSent.is_command_allowed(command::Mail("".to_string(), None))); - assert!(!super::MailSent.is_command_allowed(command::Mail("".to_string(), None))); + assert!(!super::Unconnected.is_command_allowed(&command::Noop)); + assert!(!super::DataSent.is_command_allowed(&command::Noop)); + assert!(super::HelloSent.is_command_allowed(&command::Mail("".to_string(), None))); + assert!(!super::MailSent.is_command_allowed(&command::Mail("".to_string(), None))); } #[test] fn test_next_state() { - assert_eq!(super::MailSent.next_state(command::Noop), Some(super::MailSent)); - assert_eq!(super::HelloSent.next_state(command::Mail("".to_string(), None)), + assert_eq!(super::MailSent.next_state(&command::Noop), Some(super::MailSent)); + assert_eq!(super::HelloSent.next_state(&command::Mail("".to_string(), None)), Some(super::MailSent)); - assert_eq!(super::MailSent.next_state(command::Mail("".to_string(), None)), None); + assert_eq!(super::MailSent.next_state(&command::Mail("".to_string(), None)), None); } }