Skip to content

Commit 21b334e

Browse files
sylvestresylvestre
committed
chmod dedup some code
1 parent da509a5 commit 21b334e

File tree

2 files changed

+131
-128
lines changed

2 files changed

+131
-128
lines changed

‎src/uu/chmod/src/chmod.rs‎

Lines changed: 127 additions & 128 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,117 @@ struct Chmoder {
269269
}
270270

271271
impl Chmoder {
272+
/// Calculate the new mode based on the current mode and the chmod specification.
273+
/// Returns (`new_mode`, `naively_expected_new_mode`) for symbolic modes, or (`new_mode`, `new_mode`) for numeric/reference modes.
274+
fn calculate_new_mode(&self, current_mode: u32, is_dir: bool) -> UResult<(u32, u32)> {
275+
match self.fmode {
276+
Some(mode) => Ok((mode, mode)),
277+
None => {
278+
let cmode_unwrapped = self.cmode.clone().unwrap();
279+
let mut new_mode = current_mode;
280+
let mut naively_expected_new_mode = current_mode;
281+
282+
for mode in cmode_unwrapped.split(',') {
283+
let result = if mode.chars().any(|c| c.is_ascii_digit()) {
284+
mode::parse_numeric(new_mode, mode, is_dir).map(|v| (v, v))
285+
} else {
286+
mode::parse_symbolic(new_mode, mode, mode::get_umask(), is_dir).map(|m| {
287+
// calculate the new mode as if umask was 0
288+
let naive_mode =
289+
mode::parse_symbolic(naively_expected_new_mode, mode, 0, is_dir)
290+
.unwrap(); // we know that mode must be valid, so this cannot fail
291+
(m, naive_mode)
292+
})
293+
};
294+
295+
match result {
296+
Ok((mode, naive_mode)) => {
297+
new_mode = mode;
298+
naively_expected_new_mode = naive_mode;
299+
}
300+
Err(f) => {
301+
return if self.quiet {
302+
Err(ExitCode::new(1))
303+
} else {
304+
Err(USimpleError::new(1, f))
305+
};
306+
}
307+
}
308+
}
309+
Ok((new_mode, naively_expected_new_mode))
310+
}
311+
}
312+
}
313+
314+
/// Report permission changes based on verbose and changes flags
315+
fn report_permission_change(&self, file_path: &Path, old_mode: u32, new_mode: u32) {
316+
if self.verbose || self.changes {
317+
let current_permissions = display_permissions_unix(old_mode as mode_t, false);
318+
let new_permissions = display_permissions_unix(new_mode as mode_t, false);
319+
320+
if new_mode != old_mode {
321+
if self.changes || self.verbose {
322+
println!(
323+
"mode of {} changed from {:04o} ({}) to {:04o} ({})",
324+
file_path.quote(),
325+
old_mode,
326+
current_permissions,
327+
new_mode,
328+
new_permissions
329+
);
330+
}
331+
} else if self.verbose {
332+
println!(
333+
"mode of {} retained as {:04o} ({})",
334+
file_path.quote(),
335+
old_mode,
336+
current_permissions
337+
);
338+
}
339+
}
340+
}
341+
342+
/// Handle symlinks during directory traversal based on traversal mode
343+
#[cfg(not(target_os = "linux"))]
344+
fn handle_symlink_during_traversal(
345+
&self,
346+
path: &Path,
347+
is_command_line_arg: bool,
348+
) -> UResult<()> {
349+
match self.traverse_symlinks {
350+
TraverseSymlinks::All => {
351+
// Follow all symlinks during traversal
352+
match fs::metadata(path) {
353+
Ok(meta) if meta.is_dir() => self.walk_dir_with_context(path, false),
354+
Ok(_) => {
355+
// It's a file symlink, chmod it
356+
self.chmod_file(path)
357+
}
358+
Err(_) => {
359+
// Dangling symlink, chmod it without dereferencing
360+
self.chmod_file_internal(path, false)
361+
}
362+
}
363+
}
364+
TraverseSymlinks::First => {
365+
if is_command_line_arg {
366+
// Follow symlinks that are command line args
367+
match fs::metadata(path) {
368+
Ok(meta) if meta.is_dir() => self.walk_dir_with_context(path, false),
369+
Ok(_) => self.chmod_file(path),
370+
Err(_) => self.chmod_file_internal(path, false),
371+
}
372+
} else {
373+
// Don't follow symlinks encountered during recursion
374+
self.chmod_file_internal(path, false)
375+
}
376+
}
377+
TraverseSymlinks::None => {
378+
// Don't follow symlinks
379+
self.chmod_file_internal(path, false)
380+
}
381+
}
382+
}
272383
fn chmod(&self, files: &[OsString]) -> UResult<()> {
273384
let mut r = Ok(());
274385

@@ -489,35 +600,8 @@ impl Chmoder {
489600
entry_name: &std::ffi::OsStr,
490601
current_mode: u32,
491602
) -> UResult<()> {
492-
// Determine the new permissions to apply
493-
let new_mode = match self.fmode {
494-
Some(mode) => mode,
495-
None => {
496-
let cmode_unwrapped = self.cmode.clone().unwrap();
497-
let mut new_mode = current_mode;
498-
for mode in cmode_unwrapped.split(',') {
499-
let result = if mode.chars().any(|c| c.is_ascii_digit()) {
500-
mode::parse_numeric(new_mode, mode, file_path.is_dir())
501-
} else {
502-
mode::parse_symbolic(new_mode, mode, mode::get_umask(), file_path.is_dir())
503-
};
504-
505-
match result {
506-
Ok(parsed_mode) => {
507-
new_mode = parsed_mode;
508-
}
509-
Err(f) => {
510-
return if self.quiet {
511-
Err(ExitCode::new(1))
512-
} else {
513-
Err(USimpleError::new(1, f))
514-
};
515-
}
516-
}
517-
}
518-
new_mode
519-
}
520-
};
603+
// Calculate the new mode using the helper method
604+
let (new_mode, _) = self.calculate_new_mode(current_mode, file_path.is_dir())?;
521605

522606
// Use safe traversal to change the mode
523607
let follow_symlinks = self.dereference;
@@ -534,67 +618,24 @@ impl Chmoder {
534618
);
535619
}
536620

537-
// Report the change if verbose or changes mode
538-
if self.verbose || self.changes {
539-
let current_permissions = display_permissions_unix(current_mode, false);
540-
let new_permissions = display_permissions_unix(new_mode, false);
541-
if new_mode != current_mode {
542-
if self.changes || self.verbose {
543-
println!(
544-
"mode of {} changed from {} ({:04o}) to {} ({:04o})",
545-
file_path.quote(),
546-
current_permissions,
547-
current_mode,
548-
new_permissions,
549-
new_mode
550-
);
551-
}
552-
} else if self.verbose {
553-
println!(
554-
"mode of {} retained as {} ({:04o})",
555-
file_path.quote(),
556-
current_permissions,
557-
current_mode
558-
);
559-
}
560-
}
621+
// Report the change using the helper method
622+
self.report_permission_change(file_path, current_mode, new_mode);
561623

562624
Ok(())
563625
}
564626

565627
#[cfg(not(target_os = "linux"))]
566628
fn handle_symlink_during_recursion(&self, path: &Path) -> UResult<()> {
567-
// During recursion, determine behavior based on traversal mode
568-
match self.traverse_symlinks {
569-
TraverseSymlinks::All => {
570-
// Follow all symlinks during recursion
571-
// Check if the symlink target is a directory, but handle dangling symlinks gracefully
572-
match fs::metadata(path) {
573-
Ok(meta) if meta.is_dir() => self.walk_dir_with_context(path, false),
574-
Ok(_) => {
575-
// It's a file symlink, chmod it
576-
self.chmod_file(path)
577-
}
578-
Err(_) => {
579-
// Dangling symlink, chmod it without dereferencing
580-
self.chmod_file_internal(path, false)
581-
}
582-
}
583-
}
584-
TraverseSymlinks::First | TraverseSymlinks::None => {
585-
// Don't follow symlinks encountered during recursion
586-
// For these symlinks, don't dereference them even if dereference is normally true
587-
self.chmod_file_internal(path, false)
588-
}
589-
}
629+
// Use the common symlink handling logic
630+
self.handle_symlink_during_traversal(path, false)
590631
}
591632

592633
fn chmod_file(&self, file: &Path) -> UResult<()> {
593634
self.chmod_file_internal(file, self.dereference)
594635
}
595636

596637
fn chmod_file_internal(&self, file: &Path, dereference: bool) -> UResult<()> {
597-
use uucore::{mode::get_umask, perms::get_metadata};
638+
use uucore::perms::get_metadata;
598639

599640
let metadata = get_metadata(file, dereference);
600641

@@ -620,45 +661,14 @@ impl Chmoder {
620661
}
621662
};
622663

623-
// Determine the new permissions to apply
664+
// Calculate the new mode using the helper method
665+
let (new_mode, naively_expected_new_mode) =
666+
self.calculate_new_mode(fperm, file.is_dir())?;
667+
668+
// Determine how to apply the permissions
624669
match self.fmode {
625670
Some(mode) => self.change_file(fperm, mode, file)?,
626671
None => {
627-
let cmode_unwrapped = self.cmode.clone().unwrap();
628-
let mut new_mode = fperm;
629-
let mut naively_expected_new_mode = new_mode;
630-
for mode in cmode_unwrapped.split(',') {
631-
let result = if mode.chars().any(|c| c.is_ascii_digit()) {
632-
mode::parse_numeric(new_mode, mode, file.is_dir()).map(|v| (v, v))
633-
} else {
634-
mode::parse_symbolic(new_mode, mode, get_umask(), file.is_dir()).map(|m| {
635-
// calculate the new mode as if umask was 0
636-
let naive_mode = mode::parse_symbolic(
637-
naively_expected_new_mode,
638-
mode,
639-
0,
640-
file.is_dir(),
641-
)
642-
.unwrap(); // we know that mode must be valid, so this cannot fail
643-
(m, naive_mode)
644-
})
645-
};
646-
647-
match result {
648-
Ok((mode, naive_mode)) => {
649-
new_mode = mode;
650-
naively_expected_new_mode = naive_mode;
651-
}
652-
Err(f) => {
653-
return if self.quiet {
654-
Err(ExitCode::new(1))
655-
} else {
656-
Err(USimpleError::new(1, f))
657-
};
658-
}
659-
}
660-
}
661-
662672
// Special handling for symlinks when not dereferencing
663673
if file.is_symlink() && !dereference {
664674
// TODO: On most Unix systems, symlink permissions are ignored by the kernel,
@@ -690,13 +700,8 @@ impl Chmoder {
690700

691701
fn change_file(&self, fperm: u32, mode: u32, file: &Path) -> Result<(), i32> {
692702
if fperm == mode {
693-
if self.verbose && !self.changes {
694-
println!(
695-
"mode of {} retained as {fperm:04o} ({})",
696-
file.quote(),
697-
display_permissions_unix(fperm as mode_t, false),
698-
);
699-
}
703+
// Use the helper method for consistent reporting
704+
self.report_permission_change(file, fperm, mode);
700705
Ok(())
701706
} else if let Err(err) = fs::set_permissions(file, fs::Permissions::from_mode(mode)) {
702707
if !self.quiet {
@@ -712,14 +717,8 @@ impl Chmoder {
712717
}
713718
Err(1)
714719
} else {
715-
if self.verbose || self.changes {
716-
println!(
717-
"mode of {} changed from {fperm:04o} ({}) to {mode:04o} ({})",
718-
file.quote(),
719-
display_permissions_unix(fperm as mode_t, false),
720-
display_permissions_unix(mode as mode_t, false)
721-
);
722-
}
720+
// Use the helper method for consistent reporting
721+
self.report_permission_change(file, fperm, mode);
723722
Ok(())
724723
}
725724
}

‎tests/by-util/test_chmod.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,10 @@ fn test_chmod_recursive() {
391391
make_file(&at.plus_as_string("a/b/b"), 0o100444);
392392
make_file(&at.plus_as_string("a/b/c/c"), 0o100444);
393393
make_file(&at.plus_as_string("z/y"), 0o100444);
394+
#[cfg(not(target_os = "linux"))]
395+
let err_msg = "chmod: Permission denied\n";
396+
#[cfg(target_os = "linux")]
397+
let err_msg = "chmod: 'z': Permission denied\n";
394398

395399
// only the permissions of folder `a` and `z` are changed
396400
// folder can't be read after read permission is removed

0 commit comments

Comments
 (0)