Skip to content

Commit fba43f4

Browse files
sylvestrecakebaker
authored andcommitted
improve code
1 parent 5551c6a commit fba43f4

File tree

1 file changed

+25
-24
lines changed

1 file changed

+25
-24
lines changed

src/uucore/src/lib/features/perms.rs

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -308,27 +308,23 @@ impl ChownExecutor {
308308

309309
let ret = if self.matched(meta.uid(), meta.gid()) {
310310
// Use safe syscalls for root directory to prevent TOCTOU attacks on Linux
311-
let chown_result = if cfg!(target_os = "linux") && path.is_dir() {
311+
#[cfg(target_os = "linux")]
312+
let chown_result = if path.is_dir() {
312313
// For directories on Linux, use safe traversal from the start
313-
#[cfg(target_os = "linux")]
314-
{
315-
match DirFd::open(path) {
316-
Ok(dir_fd) => self.safe_chown_dir(&dir_fd, path, &meta).map(|_| String::new()),
317-
Err(_e) => {
318-
// Don't show error here - let safe_dive_into handle directory traversal errors
319-
// This prevents duplicate error messages
320-
Ok(String::new())
321-
}
314+
match DirFd::open(path) {
315+
Ok(dir_fd) => self
316+
.safe_chown_dir(&dir_fd, path, &meta)
317+
.map(|_| String::new()),
318+
Err(_e) => {
319+
// Don't show error here - let safe_dive_into handle directory traversal errors
320+
// This prevents duplicate error messages
321+
Ok(String::new())
322322
}
323323
}
324324
} else {
325325
// For non-directories (files, symlinks), use the regular wrap_chown method
326-
#[cfg(not(target_os = "linux"))]
327-
{
328-
unreachable!()
329-
}
330326
wrap_chown(
331-
// For non-directories (files, symlinks) or non-Linux systems, use the regular wrap_chown method
327+
path,
332328
&meta,
333329
self.dest_uid,
334330
self.dest_gid,
@@ -337,6 +333,16 @@ impl ChownExecutor {
337333
)
338334
};
339335

336+
#[cfg(not(target_os = "linux"))]
337+
let chown_result = wrap_chown(
338+
path,
339+
&meta,
340+
self.dest_uid,
341+
self.dest_gid,
342+
self.dereference,
343+
self.verbosity.clone(),
344+
);
345+
340346
match chown_result {
341347
Ok(n) => {
342348
if !n.is_empty() {
@@ -372,15 +378,10 @@ impl ChownExecutor {
372378
} else {
373379
ret
374380
}
375-
) -> Result<(), String> {
381+
}
376382

377383
#[cfg(target_os = "linux")]
378-
fn safe_chown_dir(
379-
&self,
380-
dir_fd: &DirFd,
381-
path: &Path,
382-
meta: &Metadata,
383-
) -> Result<String, String> {
384+
fn safe_chown_dir(&self, dir_fd: &DirFd, path: &Path, meta: &Metadata) -> Result<(), String> {
384385
let dest_uid = self.dest_uid.unwrap_or_else(|| meta.uid());
385386
let dest_gid = self.dest_gid.unwrap_or_else(|| meta.gid());
386387

@@ -417,15 +418,15 @@ impl ChownExecutor {
417418
entries::uid2usr(dest_uid).unwrap_or_else(|_| dest_uid.to_string()),
418419
entries::gid2grp(dest_gid).unwrap_or_else(|_| dest_gid.to_string())
419420
)
420-
Ok(())
421+
};
421422
}
422423

423424
return Err(error_msg);
424425
}
425426

426427
// Report the change if verbose (similar to wrap_chown)
427428
self.report_ownership_change_success(path, meta.uid(), meta.gid());
428-
Ok(String::new())
429+
Ok(())
429430
}
430431

431432
#[cfg(target_os = "linux")]

0 commit comments

Comments
 (0)