Skip to content

Commit cd1147d

Browse files
mattsu2020sylvestre
authored andcommitted
refactor(mv): enhance error handling in ownership and permission preservation
- Capture return values from chown/lchown and set_permissions calls - Emit warnings for non-permission errors to inform users of preservation failures - Keep operations non-fatal as preservation is best-effort on Unix systems
1 parent e747495 commit cd1147d

File tree

1 file changed

+32
-4
lines changed

1 file changed

+32
-4
lines changed

src/uu/mv/src/mv.rs

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -804,6 +804,13 @@ fn is_fifo(_filetype: fs::FileType) -> bool {
804804
}
805805

806806
#[cfg(unix)]
807+
/// Best-effort ownership preservation for `to` using `from_meta`.
808+
///
809+
/// On Unix, this tries to set `uid`/`gid` on `to`. If `follow_symlinks` is
810+
/// true it uses `chown`, otherwise it uses `lchown` so the link itself (not its
811+
/// target) is updated. `chown`/`lchown` failures are non-fatal; permission
812+
/// errors are ignored, and other failures emit a warning because ownership
813+
/// preservation is optional.
807814
fn try_preserve_ownership(from_meta: &fs::Metadata, to: &Path, follow_symlinks: bool) {
808815
use std::ffi::CString;
809816
use std::os::unix::ffi::OsStrExt as _;
@@ -816,22 +823,43 @@ fn try_preserve_ownership(from_meta: &fs::Metadata, to: &Path, follow_symlinks:
816823
return;
817824
};
818825

819-
unsafe {
826+
let result = unsafe {
820827
if follow_symlinks {
821-
let _ = libc::chown(to_cstr.as_ptr(), uid, gid);
828+
libc::chown(to_cstr.as_ptr(), uid, gid)
822829
} else {
823-
let _ = libc::lchown(to_cstr.as_ptr(), uid, gid);
830+
libc::lchown(to_cstr.as_ptr(), uid, gid)
831+
}
832+
};
833+
if result != 0 {
834+
let err = io::Error::last_os_error();
835+
if err.kind() != io::ErrorKind::PermissionDenied {
836+
eprintln!(
837+
"mv: warning: failed to preserve ownership for {}: {err}",
838+
to.quote()
839+
);
824840
}
825841
}
826842
}
827843

828844
#[cfg(unix)]
845+
/// Best-effort permission preservation for `to` using `from_meta`.
846+
///
847+
/// Only the mode bits are applied (`chmod` does not accept file type bits).
848+
/// Failures are non-fatal; permission errors are ignored, and other failures
849+
/// emit a warning because this is optional.
829850
fn try_preserve_permissions(from_meta: &fs::Metadata, to: &Path) {
830851
use std::os::unix::fs::{MetadataExt as _, PermissionsExt as _};
831852

832853
// Keep mode bits only (file type bits are not allowed in chmod).
833854
let mode = from_meta.mode() & 0o7777;
834-
let _ = fs::set_permissions(to, fs::Permissions::from_mode(mode));
855+
if let Err(err) = fs::set_permissions(to, fs::Permissions::from_mode(mode)) {
856+
if err.kind() != io::ErrorKind::PermissionDenied {
857+
eprintln!(
858+
"mv: warning: failed to preserve permissions for {}: {err}",
859+
to.quote()
860+
);
861+
}
862+
}
835863
}
836864

837865
#[cfg(unix)]

0 commit comments

Comments
 (0)