Skip to content

Commit

Permalink
Avoid reallocating Strings when creating TarError (#269)
Browse files Browse the repository at this point in the history
  • Loading branch information
vrmiguel committed Oct 28, 2021
1 parent 1890555 commit 8333ef6
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 12 deletions.
2 changes: 1 addition & 1 deletion src/archive.rs
Expand Up @@ -189,7 +189,7 @@ impl Archive<dyn Read + '_> {
fn _unpack(&mut self, dst: &Path) -> io::Result<()> {
if dst.symlink_metadata().is_err() {
fs::create_dir_all(&dst)
.map_err(|e| TarError::new(&format!("failed to create `{}`", dst.display()), e))?;
.map_err(|e| TarError::new(format!("failed to create `{}`", dst.display()), e))?;
}

// Canonicalizing the dst directory will prepend the path with '\\?\'
Expand Down
16 changes: 8 additions & 8 deletions src/entry.rs
Expand Up @@ -379,7 +379,7 @@ impl<'a> EntryFields<'a> {
{
let path = self.path().map_err(|e| {
TarError::new(
&format!("invalid path in entry header: {}", self.path_lossy()),
format!("invalid path in entry header: {}", self.path_lossy()),
e,
)
})?;
Expand Down Expand Up @@ -414,12 +414,12 @@ impl<'a> EntryFields<'a> {
};

self.ensure_dir_created(&dst, parent)
.map_err(|e| TarError::new(&format!("failed to create `{}`", parent.display()), e))?;
.map_err(|e| TarError::new(format!("failed to create `{}`", parent.display()), e))?;

let canon_target = self.validate_inside_dst(&dst, parent)?;

self.unpack(Some(&canon_target), &file_dst)
.map_err(|e| TarError::new(&format!("failed to unpack `{}`", file_dst.display()), e))?;
.map_err(|e| TarError::new(format!("failed to unpack `{}`", file_dst.display()), e))?;

Ok(true)
}
Expand Down Expand Up @@ -607,7 +607,7 @@ impl<'a> EntryFields<'a> {
.map_err(|e| {
let header = self.header.path_bytes();
TarError::new(
&format!(
format!(
"failed to unpack `{}` into `{}`",
String::from_utf8_lossy(&header),
dst.display()
Expand All @@ -627,7 +627,7 @@ impl<'a> EntryFields<'a> {
let mtime = if mtime == 0 { 1 } else { mtime };
let mtime = FileTime::from_unix_time(mtime as i64, 0);
filetime::set_file_handle_times(&f, Some(mtime), Some(mtime)).map_err(|e| {
TarError::new(&format!("failed to set mtime for `{}`", dst.display()), e)
TarError::new(format!("failed to set mtime for `{}`", dst.display()), e)
})?;
}
}
Expand All @@ -647,7 +647,7 @@ impl<'a> EntryFields<'a> {
) -> Result<(), TarError> {
_set_perms(dst, f, mode, preserve).map_err(|e| {
TarError::new(
&format!(
format!(
"failed to set permissions to {:o} \
for `{}`",
mode,
Expand Down Expand Up @@ -735,7 +735,7 @@ impl<'a> EntryFields<'a> {
for (key, value) in exts {
xattr::set(dst, key, value).map_err(|e| {
TarError::new(
&format!(
format!(
"failed to set extended \
attributes to {}. \
Xattrs: key={:?}, value={:?}.",
Expand Down Expand Up @@ -794,7 +794,7 @@ impl<'a> EntryFields<'a> {
})?;
if !canon_parent.starts_with(&canon_target) {
let err = TarError::new(
&format!(
format!(
"trying to unpack outside of destination path: {}",
canon_target.display()
),
Expand Down
7 changes: 4 additions & 3 deletions src/error.rs
@@ -1,17 +1,18 @@
use std::borrow::Cow;
use std::error;
use std::fmt;
use std::io::{self, Error};

#[derive(Debug)]
pub struct TarError {
desc: String,
desc: Cow<'static, str>,
io: io::Error,
}

impl TarError {
pub fn new(desc: &str, err: Error) -> TarError {
pub fn new(desc: impl Into<Cow<'static, str>>, err: Error) -> TarError {
TarError {
desc: desc.to_string(),
desc: desc.into(),
io: err,
}
}
Expand Down

0 comments on commit 8333ef6

Please sign in to comment.