From 127cff38809ea6c3a37ed3ce7f428cafdc38e2e1 Mon Sep 17 00:00:00 2001 From: Rob Landley Date: Thu, 8 Jul 2021 04:30:00 -0500 Subject: Work around a posix violation in the croups filesystem that LTP requires. Posix says you removing a non-empty directory "shall fail" in both: https://pubs.opengroup.org/onlinepubs/9699919799.2018edition/functions/unlinkat.html https://pubs.opengroup.org/onlinepubs/9699919799.2018edition/functions/rmdir.html So toybox mv went ahead and unlinked the directory even if the contents hadn't all been deleted because posix guarantees it to be harmless. But cgroups (https://lwn.net/Articles/679786/) deletes the non-empty directory, thus the Linux Test Project's cgroups_fj_function test6 was failing with toybox mv because they depend on not triggering the posix violating behavior. Work around it by having mv DIRTREE_SAVE failing nodes and then check for a non-empty ->child in the COMEAGAIN as a signal not to unlink the dir. While I'm there do some code cleanup, add a cp -i test... --- lib/dirtree.c | 1 - tests/cp.test | 8 ++++++++ toys/posix/cp.c | 62 +++++++++++++++++++++++++-------------------------------- 3 files changed, 35 insertions(+), 36 deletions(-) diff --git a/lib/dirtree.c b/lib/dirtree.c index 43ecbd85..4023101e 100644 --- a/lib/dirtree.c +++ b/lib/dirtree.c @@ -162,7 +162,6 @@ int dirtree_recurse(struct dirtree *node, // according to the fddir() man page, the filehandle in the DIR * can still // be externally used by things that don't lseek() it. - // The extra parentheses are to shut the stupid compiler up. while ((entry = readdir(dir))) { if ((flags&DIRTREE_PROC) && !isdigit(*entry->d_name)) continue; if (!(new = dirtree_add_node(node, entry->d_name, flags))) continue; diff --git a/tests/cp.test b/tests/cp.test index b0d5d3a4..3e25f6c6 100755 --- a/tests/cp.test +++ b/tests/cp.test @@ -140,6 +140,14 @@ testing '-u1' 'echo one>one; sleep .1; echo two>two; cp -u one two; cat two' \ 'two\n' '' '' testing '-u2' 'echo two>two; sleep .1; echo one>one; cp -u one two; cat two' \ 'one\n' '' '' +mkdir a b + +echo potato > a/one +echo potato > a/two +touch b/one b/two +testing '-i' 'cp -ri a/. b/. 2>/dev/null; cmp -s a/one b/one || cmp -s a/one b/two && echo yes' \ + 'yes\n' '' 'n\ny\n' +rm -rf one two a b # cp -r ../source destdir # cp -r one/two/three missing diff --git a/toys/posix/cp.c b/toys/posix/cp.c index 8eaecc9c..ed62f04d 100644 --- a/toys/posix/cp.c +++ b/toys/posix/cp.c @@ -124,9 +124,10 @@ struct cp_preserve { static int cp_node(struct dirtree *try) { int fdout = -1, cfd = try->parent ? try->parent->extra : AT_FDCWD, + save = DIRTREE_SAVE*(CFG_MV && toys.which->name[0] == 'm'), rc = 0, tfd = dirtree_parentfd(try); unsigned flags = toys.optflags; - char *catch = try->parent ? try->name : TT.destname, *err = "%s"; + char *s = 0, *catch = try->parent ? try->name : TT.destname, *err = "%s"; struct stat cst; if (!dirtree_notdotdot(try)) return 0; @@ -135,8 +136,13 @@ static int cp_node(struct dirtree *try) if (S_ISDIR(try->st.st_mode) && try->again) { fdout = try->extra; err = 0; - } else { + // If mv child had a problem, free data and don't try to delete parent dir. + if (try->child) { + save = 0; + llist_traverse(try->child, free); + } + } else { // -d is only the same as -r for symlinks, not for directories if (S_ISLNK(try->st.st_mode) && (flags & FLAG_d)) flags |= FLAG_r; @@ -150,35 +156,27 @@ static int cp_node(struct dirtree *try) error_msg("'%s' is '%s'", catch, err = dirtree_path(try, 0)); free(err); - return 0; + return save; } // Handle -inuvF - if (!faccessat(cfd, catch, F_OK, 0) && !S_ISDIR(cst.st_mode)) { - char *s; - - if (S_ISDIR(try->st.st_mode)) { + if (S_ISDIR(try->st.st_mode)) error_msg("dir at '%s'", s = dirtree_path(try, 0)); - free(s); - return 0; - } else if ((flags & FLAG_F) && unlinkat(cfd, catch, 0)) { + else if ((flags & FLAG_F) && unlinkat(cfd, catch, 0)) error_msg("unlink '%s'", catch); - return 0; - } else if (flags & FLAG_n) return 0; - else if ((flags & FLAG_u) && nanodiff(&try->st.st_mtim, &cst.st_mtim)>0) - return 0; else if (flags & FLAG_i) { fprintf(stderr, "%s: overwrite '%s'", toys.which->name, s = dirtree_path(try, 0)); - free(s); - if (!yesno(0)) return 0; - } + if (yesno(0)) rc++; + } else if (!((flags&FLAG_u) && nanodiff(&try->st.st_mtim, &cst.st_mtim)>0) + && !(flags & FLAG_n)) rc++; + free(s); + if (!rc) return save; } if (flags & FLAG_v) { - char *s = dirtree_path(try, 0); - printf("%s '%s'\n", toys.which->name, s); + printf("%s '%s'\n", toys.which->name, s = dirtree_path(try, 0)); free(s); } @@ -246,13 +244,11 @@ static int cp_node(struct dirtree *try) } else if (!S_ISREG(try->st.st_mode) && (try->parent || (flags & (FLAG_a|FLAG_P|FLAG_r)))) { - int i; - // make symlink, or make block/char/fifo/socket if (S_ISLNK(try->st.st_mode) - ? ((i = readlinkat0(tfd, try->name, toybuf, sizeof(toybuf))) && - ((!unlinkat(cfd, catch, 0) || ENOENT == errno) && - !symlinkat(toybuf, cfd, catch))) + ? readlinkat0(tfd, try->name, toybuf, sizeof(toybuf)) && + (!unlinkat(cfd, catch, 0) || ENOENT == errno) && + !symlinkat(toybuf, cfd, catch) : !mknodat(cfd, catch, try->st.st_mode, try->st.st_rdev)) { err = 0; @@ -307,8 +303,6 @@ static int cp_node(struct dirtree *try) // Did we make a thing? if (fdout != -1) { - int rc; - // Inability to set --preserve isn't fatal, some require root access. // ownership @@ -342,23 +336,21 @@ static int cp_node(struct dirtree *try) xclose(fdout); } - if (CFG_MV && toys.which->name[0] == 'm') + if (save) if (unlinkat(tfd, try->name, S_ISDIR(try->st.st_mode) ? AT_REMOVEDIR :0)) err = "%s"; } if (err) { - char *f = 0; - if (catch == try->name) { - f = dirtree_path(try, 0); + s = dirtree_path(try, 0); while (try->parent) try = try->parent; - catch = xmprintf("%s%s", TT.destname, f+strlen(try->name)); - free(f); - f = catch; - } + catch = xmprintf("%s%s", TT.destname, s+strlen(try->name)); + free(s); + s = catch; + } else s = 0; perror_msg(err, catch); - free(f); + free(s); } return 0; } -- cgit v1.2.3