From 74cb4f87d1c880c4f53e79eec739ab082edd9ccc Mon Sep 17 00:00:00 2001 From: Daniel Hofstetter Date: Tue, 11 Aug 2026 11:06:52 +0200 Subject: [PATCH] chmod: remove duplicate code handling symlinks --- src/uu/chmod/src/chmod.rs | 62 ++++++++++++++++----------------------- 1 file changed, 26 insertions(+), 36 deletions(-) diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index 8a96aacb7f..4aeb000fc7 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -828,44 +828,34 @@ impl Chmoder { let (new_mode, naively_expected_new_mode) = self.calculate_new_mode(fperm, file.is_dir())?; - // Determine how to apply the permissions - if let Some(mode) = self.fmode { - // A symlink reached without dereferencing (for example one met while - // walking a `-R` tree) must be left alone: chmod(2) follows the link, - // so changing it would change the mode of the referent, which can live - // outside the tree. The symbolic/numeric path below already skips it. - if file.is_symlink() && !dereference { - if self.verbose { - Self::print_neither_changed(file.into())?; - } - } else { - self.change_file(fperm, mode, file)?; + // A symlink reached without dereferencing (for example one met while + // walking a `-R` tree) must be left alone: chmod(2) follows the link, + // so changing it would change the mode of the referent, which can live + // outside the tree. On most Unix systems, symlink permissions are + // ignored by the kernel anyway, so changing them has no effect. + if file.is_symlink() && !dereference { + if self.verbose { + Self::print_neither_changed(file.into())?; } } else { - // Special handling for symlinks when not dereferencing - if file.is_symlink() && !dereference { - // TODO: On most Unix systems, symlink permissions are ignored by the kernel, - // so changing them has no effect. We skip this operation for compatibility. - // Note that "chmod without dereferencing" effectively does nothing on symlinks. - if self.verbose { - Self::print_neither_changed(file.into())?; - } - } else { - self.change_file(fperm, new_mode, file)?; - } - // A bare mode such as `-w` is umask-relative, so the umask can keep permissions that - // the user asked to drop. GNU reports that as an error, but only when the mode was - // written in the option-like form (`chmod -w f`), where it doubles as a hint that the - // argument was consumed as a mode. After `--` the operand is unambiguous and GNU stays - // silent, so the diagnostic is suppressed here too. - if self.option_like_mode && (new_mode & !naively_expected_new_mode) != 0 { - return Err(ChmodError::NewPermissions( - file.into(), - display_permissions_unix(new_mode, false), - display_permissions_unix(naively_expected_new_mode, false), - ) - .into()); - } + self.change_file(fperm, self.fmode.unwrap_or(new_mode), file)?; + } + + // A bare mode such as `-w` is umask-relative, so the umask can keep permissions that + // the user asked to drop. GNU reports that as an error, but only when the mode was + // written in the option-like form (`chmod -w f`), where it doubles as a hint that the + // argument was consumed as a mode. After `--` the operand is unambiguous and GNU stays + // silent, so the diagnostic is suppressed here too. + if self.fmode.is_none() + && self.option_like_mode + && (new_mode & !naively_expected_new_mode) != 0 + { + return Err(ChmodError::NewPermissions( + file.into(), + display_permissions_unix(new_mode, false), + display_permissions_unix(naively_expected_new_mode, false), + ) + .into()); } Ok(())