From 08e60046ea5b3a6685dd260f3406ea8f02765123 Mon Sep 17 00:00:00 2001 From: Philippe Benard Date: Sat, 14 Jun 2025 08:07:11 +0200 Subject: [PATCH] Bug-871: mv builtins cannot move on cross-mountpoint, cp obscure error when -r is not given on src dir. - src/lib/libcmd/cp.c If src is a dir and -r was not given, warn with a message like -r not specified; omitting directory This fix bogus error message like cp: warning: t: directory -- copying as plain file cp: t: /tmp/t read error [Is a directory] This is not the exact purpose of the bug-871 submission, but it is in the same vein. - src/lib/libcmd/mv.c The builtin mv is unable to deal with cross-mounted file system. The code still has a -x option for 'may be' cross-mounted file system but it doesn't works. The effect of $ /opt/ast/bin/mv bin /dev/shm mv: bin: /dev/shm/bin read error [Is a directory] Is a failure on xdev mv, and a left over regular file (not dir) named. '/tmp/bin' Then $ /opt/ast/bin/cp -r bin /dev/shm cp: /dev/shm/bin: not a directory -- bin ignored Since /tmp/bin is a regular file we got the above error. The code is pretty obscure and wrong, to avoid upsetting this instable design, I propose a workaround instead of a fix. The idea is to detect xdev mounted target dst and do a cp -r src... dst If all goes well do rm -rf src... NOTE : The bug-871 report the problem with /tmp or /dev/shm but it is not related to those FS, using another disc (FS) fail the same way. The workaround. in b_mv() we try to figure out if some src... dev don't match the dts dev, if so we craft an argc argv and call b_cp() with argv[0]="XM", i.e a signature not used by b_cp() which can be called with the signature "cp", "ln", "mv". In b_cp() we detect the "XM" signature, and restore a valid "cp" signature, this signature is used by cmdinit() to set error_info.id="cp", i.e the basename of argv[0], as side effect, used for error reporting in "cp". After the cmdinit() we keep a backup of the cmd because the behavior of the code depend on switch (error_info.id[0]), and since our XM "cp" is actually a run on "mv" behalf, we set error_info.id to "mv" that is important for accurate error reporting, and now the switc become switch(cmd[0]) to still work as a cp but with an mv error report. Down the code, while looping on src... if the cmd[0]=='c' i.e cp mode, and src is a dir and -r was not given, we produce the more conventional error message "-r not specified; omitting directory %s",src --- src/cmd/ksh93/tests/libcmd.sh | 88 +++++++++++++++++++++++++++++++++++ src/lib/libcmd/cp.c | 46 +++++++++++++++++- src/lib/libcmd/mv.c | 65 ++++++++++++++++++++++++++ 3 files changed, 197 insertions(+), 2 deletions(-) diff --git a/src/cmd/ksh93/tests/libcmd.sh b/src/cmd/ksh93/tests/libcmd.sh index 62138642645a..d499d9fad441 100755 --- a/src/cmd/ksh93/tests/libcmd.sh +++ b/src/cmd/ksh93/tests/libcmd.sh @@ -930,5 +930,93 @@ if builtin cp 2>/dev/null; then "(got status $e$( ((e>128)) && print -n /SIG && kill -l "$e"), $(printf %q "$got"))" fi +# ====== +# https://github.com/ksh93/ksh/issues/871 +# The mv test require 2 different mounted FS, simce $tmp is located into /tmp +# we will create yet another tmp located in the artefact dir ${SHELL%/dyn/*} +T2=${LD_LIBRARY_PATH%/dyn/*}/tmp +rm -rf $T2 +mkdir -p $T2 + +rm -rf a b t u v w >/dev/null 2>&1 ; mkdir t v ; echo T>t/T U>u V>v/V W>w + +builtin cp 2>/dev/null && +builtin mv 2>/dev/null && +{ + got=$(cp t x 2>&1) + exp="cp: warning: -r not specified; omitting directory t" + [ "$got" = "$exp" ] || err_exit "builtin cp: exp '$exp' got '$got'" + + cp -r [tuvw] $T2 + got=$( find $T2/[tuvw] | sed "s:$T2/::") + exp=$(find [tuvw]) + [ "$got" = "$exp" ] || err_exit "builtin cp: exp '$exp' got '$got'" + rm -rf $T2/[tuvw] + +# Bogus builtin error message on non existant targt path +# Even the -h doesn't works as specified (create target dir's) +# So here we expect the bogus error message in lieu of +# mv: target '/tmp/d1/d2': No such file or directory + exp1="$exp" + got=$(mv [tuvw] $T2/d1/d2 2>&1) + exp=\ +'Usage: mv [-aphHlULPdfirRsuvFbxX] [-A eipt] [-B[type]] [-S suffix] source + destination + Or: mv file ... directory + Help: mv [ --help | --man ]' + [ "$got" = "$exp" ] || err_exit "builtin mv: exp '$exp' got '$got'" + +# Check the above expectde failure didn't destroyed [tuvw] + got=$(find [tuvw]) + exp="$exp1" + [ "$got" = "$exp" ] || err_exit "builtin mv: src destoyed: exp '$exp' got '$got'" + +# missing file along the args, should cause failure and no src... destruction. + rm -rf z >/dev/null 2>&1 ; mkdir z + got=$(mv t a u u/m v w z 2>&1) + exp=\ +'mv: a: not found +mv: u/m: not found +mv: warning: src not removed due to errors' + [ "$got" = "$exp" ] || err_exit "builtin mv: exp '$exp' got '$got'" + +# Check the above expected failure didn't destroyed [tuvw] + got=$(find [tuvw]) + exp="$exp1" + [ "$got" = "$exp" ] || err_exit "builtin mv: src destoyed: exp '$exp' got '$got'" + +# mv files and dirs into the same mount point. + rm -rf z >/dev/null 2>&1 ; mkdir z + got=$(mv t u v w z 2>&1) + exp='' + [ "$got" = "$exp" ] || err_exit "builtin mv: exp '$exp' got '$got'" + +# mv files back same mount point. + got=$(mv z/* . 2>&1 ; find [tuvw]) + exp="$exp1" + [ "$got" = "$exp" ] || err_exit "builtin mv: restore failed: exp '$exp' got '$got'" + +# mv files and dirs into the xdev mount point. + rm -rf $T2/z >/dev/null 2>&1 ; mkdir $T2/z + got=$(mv t u v w $T2/z 2>&1) + exp='' + [ "$got" = "$exp" ] || err_exit "builtin mv xdev: exp '$exp' got '$got'" + +# Check src's removed. + got=$(find [tuvw] 2>&1) got="${got##*:}" + exp=" No such file or directory" + [ "$got" = "$exp" ] || err_exit "builtin mv xdev, src's left over: exp '$exp' got '$got'" + +# Check dst + got=$(find $T2/z/[tuvw] 2>&1 | sed "s:$T2/z/::" ) + exp="$exp1" + [ "$got" = "$exp" ] || err_exit "builtin mv xdev, dst wrong: exp '$exp' got '$got'" + + + +} + + + # ====== exit $((Errors<125?Errors:125)) diff --git a/src/lib/libcmd/cp.c b/src/lib/libcmd/cp.c index 37f6f842c11a..908ceab9e9bc 100644 --- a/src/lib/libcmd/cp.c +++ b/src/lib/libcmd/cp.c @@ -677,8 +677,31 @@ b_cp(int argc, char** argv, Shbltin_t* context) State_t* state; Shbltin_t* sh; Shbltin_t* cleanup = context; + char* cmd; /* bug-871: Phi: */ + int xmv=0; /* bug-871: Phi: */ + /* bug-871: Phi: b_mv() want cp -r on xdev*/ + if(argv[0][0]=='X') + { + argv[0]="cp"; + xmv=1; + } cmdinit(argc, argv, context, ERROR_CATALOG, ERROR_NOTIFY); + + /* + * bug-871: Phi: + * The above cmdinit() do set error_info.id to cp|mv|ln from argv[0] + * If we are called from b_mv() in the xdev case, we keep the + * current error_id ("cp") int cmd, then we restore the error_info.id + * to "mv" for accurate error message reporting. + * The backup'ed error_info.id in cmd is then used down. + * b_mv() in the non xdev (same FS) call b_cp() with argv[0]="mv" in + * which case we don't enter the new code (cp -r; rm ) + */ + cmd=error_info.id; + if(xmv) + error_info.id="mv"; + if (!(sh = CMD_CONTEXT(context)) || !(state = (State_t*)sh->ptr)) { if (!(state = newof(0, State_t, 1, 0))) @@ -704,7 +727,7 @@ b_cp(int argc, char** argv, Shbltin_t* context) } sfputr(state->tmp, usage_head, -1); standard = !!conformance(0, 0); - switch (error_info.id[0]) + switch (cmd[0]) /* bug-871: Phi: */ { case 'c': case 'C': @@ -989,7 +1012,26 @@ b_cp(int argc, char** argv, Shbltin_t* context) state->flags |= FTS_TOP; if (fts = fts_open(argv, state->flags, NULL)) { - while (!sh_checksig(context) && (ent = fts_read(fts)) && !visit(state, ent)); + /* bug-871: Phi: */ + while (!sh_checksig(context) && (ent = fts_read(fts))) + { + /* + * bug-871: Phi: + * If src is a dir and -r was not given, skip src + */ + if( cmd[0]=='c' && + S_ISDIR(ent->fts_statp->st_mode) && + !state->recursive ) + { + error(1, + "-r not specified; omitting directory %s", + ent->fts_path); + fts_read(fts); + continue; + } + if(visit(state, ent)) + break; + } fts_close(fts); } else if (state->link != pathsetlink) diff --git a/src/lib/libcmd/mv.c b/src/lib/libcmd/mv.c index c67805d14b4d..9a0f196a9c02 100644 --- a/src/lib/libcmd/mv.c +++ b/src/lib/libcmd/mv.c @@ -27,5 +27,70 @@ int b_mv(int argc, char** argv, Shbltin_t* context) { + /* + * bug-871: Phi: Catch xdev mv + * If any of the src is not on same dev as the dst one it is an + * xdev move and b_cp() in MV mode don't handle it correctly. + * So the gross hack here is a work around, on xdev move + * we do a cp -r src... dst (which seems to work) + * then on success we do a rm -r src... + * On degenerate case, i.e case where we can not figure out the dst dev + * we go the xdev path (cp;rm) and let cp deal with errors. + */ + int ac; + char **av; + struct stat sts, std; + int i; + int xdev=0; + + if(argc>2) + { + if(stat(argv[argc-1], &std)) + { std.st_dev=0; + } + for(i=1;i