Skip to content

Commit fda3fc6

Browse files
Manciukicroypat
authored andcommitted
refactor: remove unnecessary &mut PathBuf
There's no need to have the PathBuf mutables and pass them around, as we can use them as immutables and copy them on need with `join`. Signed-off-by: Riccardo Mancini <[email protected]>
1 parent 3afe22a commit fda3fc6

File tree

1 file changed

+12
-27
lines changed

1 file changed

+12
-27
lines changed

src/jailer/src/cgroup.rs

Lines changed: 12 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ impl CgroupConfiguration {
277277
// the file no longer being empty, regardless of who actually got to populated its contents.
278278

279279
fn inherit_from_parent_aux(
280-
path: &mut PathBuf,
280+
path: &Path,
281281
file_name: &str,
282282
retry_depth: u16,
283283
) -> Result<(), JailerError> {
@@ -296,7 +296,7 @@ fn inherit_from_parent_aux(
296296

297297
// Trying to avoid the race condition described above. We don't care about the result,
298298
// because we check once more if line.is_empty() after the end of this block.
299-
let _ = inherit_from_parent_aux(&mut parent.to_path_buf(), file_name, retry_depth - 1);
299+
let _ = inherit_from_parent_aux(parent, file_name, retry_depth - 1);
300300
line = readln_special(&parent_file)?;
301301
}
302302

@@ -308,16 +308,12 @@ fn inherit_from_parent_aux(
308308
}
309309
}
310310

311-
path.push(file_name);
312-
writeln_special(&path, &line)?;
313-
path.pop();
311+
writeln_special(&path.join(file_name), &line)?;
314312

315313
Ok(())
316314
}
317315

318-
// The path reference is &mut here because we do a push to get the destination file name. However,
319-
// a pop follows shortly after (see fn inherit_from_parent_aux), reverting to the original value.
320-
fn inherit_from_parent(path: &mut PathBuf, file_name: &str, depth: u16) -> Result<(), JailerError> {
316+
fn inherit_from_parent(path: &Path, file_name: &str, depth: u16) -> Result<(), JailerError> {
321317
inherit_from_parent_aux(path, file_name, depth)
322318
}
323319

@@ -362,19 +358,15 @@ impl Cgroup for CgroupV1 {
362358
}
363359

364360
fn write_values(&self) -> Result<(), JailerError> {
365-
let location = &mut self.base.location.clone();
366-
367361
// Create the cgroup directory for the controller.
368362
fs::create_dir_all(&self.base.location)
369363
.map_err(|err| JailerError::CreateDir(self.base.location.clone(), err))?;
370364

371365
for property in self.base.properties.iter() {
372-
let file_location = &mut location.clone();
373366
// Write the corresponding cgroup value. inherit_from_parent is used to
374367
// correctly propagate the value if not defined.
375-
inherit_from_parent(location, &property.file, self.cg_parent_depth)?;
376-
file_location.push(&property.file);
377-
writeln_special(file_location, &property.value)?;
368+
inherit_from_parent(&self.base.location, &property.file, self.cg_parent_depth)?;
369+
writeln_special(&self.base.location.join(&property.file), &property.value)?;
378370
}
379371

380372
Ok(())
@@ -466,15 +458,14 @@ impl Cgroup for CgroupV2 {
466458
}
467459

468460
fn write_values(&self) -> Result<(), JailerError> {
469-
let location = &mut self.base.location.clone();
470461
let mut enabled_controllers: HashSet<&str> = HashSet::new();
471462

472463
// Create the cgroup directory for the controller.
473464
fs::create_dir_all(&self.base.location)
474465
.map_err(|err| JailerError::CreateDir(self.base.location.clone(), err))?;
475466

476467
// Ok to unwrap since the path was just created.
477-
let parent = location.parent().unwrap();
468+
let parent = self.base.location.parent().unwrap();
478469

479470
for property in self.base.properties.iter() {
480471
let controller = get_controller_from_filename(&property.file)?;
@@ -484,9 +475,7 @@ impl Cgroup for CgroupV2 {
484475
CgroupV2::write_all_subtree_control(parent, controller)?;
485476
enabled_controllers.insert(controller);
486477
}
487-
let file_location = &mut location.clone();
488-
file_location.push(&property.file);
489-
writeln_special(file_location, &property.value)?;
478+
writeln_special(&self.base.location.join(&property.file), &property.value)?;
490479
}
491480

492481
Ok(())
@@ -833,8 +822,8 @@ mod tests {
833822
let dir = TempDir::new().expect("Cannot create temporary directory.");
834823
// This is /A/B/C .
835824
let dir2 = TempDir::new_in(dir.as_path()).expect("Cannot create temporary directory.");
836-
let mut path2 = PathBuf::from(dir2.as_path());
837-
let result = inherit_from_parent(&mut PathBuf::from(&path2), "inexistent", 1);
825+
let path2 = PathBuf::from(dir2.as_path());
826+
let result = inherit_from_parent(&path2, "inexistent", 1);
838827
assert!(
839828
matches!(result, Err(JailerError::ReadToString(_, _))),
840829
"{:?}",
@@ -844,11 +833,7 @@ mod tests {
844833
// 2. If parent file exists and is empty, will go one level up, and return error because
845834
// the grandparent file does not exist.
846835
let named_file = TempFile::new_in(dir.as_path()).expect("Cannot create named file.");
847-
let result = inherit_from_parent(
848-
&mut path2.clone(),
849-
named_file.as_path().to_str().unwrap(),
850-
1,
851-
);
836+
let result = inherit_from_parent(&path2, named_file.as_path().to_str().unwrap(), 1);
852837
assert!(
853838
matches!(result, Err(JailerError::CgroupInheritFromParent(_, _))),
854839
"{:?}",
@@ -861,7 +846,7 @@ mod tests {
861846
// contents.
862847
let some_line = "Parent line";
863848
writeln!(named_file.as_file(), "{}", some_line).expect("Cannot write to file.");
864-
let result = inherit_from_parent(&mut path2, named_file.as_path().to_str().unwrap(), 1);
849+
let result = inherit_from_parent(&path2, named_file.as_path().to_str().unwrap(), 1);
865850
result.unwrap();
866851
let res = readln_special(&child_file).expect("Cannot read from file.");
867852
assert!(res == some_line);

0 commit comments

Comments
 (0)