[Toybox] [PATCH] Fix mv on overwrite.

enh enh at google.com
Fri Aug 28 18:02:33 PDT 2015


two patches here. the first patch is clearly a desirable bug fix but
there are some trickier compatibility choices lurking in the same
area. there's a bigger patch below, but i've provided both in case the
latter requires too much thought/experimentation :-)

(facebook found this bug:
https://code.google.com/p/android-developer-preview/issues/detail?id=3096)





Fix mv on overwrite.

We need to remove the destination, not the source, to be able to overwrite.

diff --git a/tests/mv.test b/tests/mv.test
index 53fc999..030e9cc 100755
--- a/tests/mv.test
+++ b/tests/mv.test
@@ -96,3 +96,10 @@ touch file1 file2
 testing "Move -n file new_file (exist)" "mv -n file1 file2 &&
    [ -e file1 -a -e file2 ] && echo 'yes'" "yes\n" "" ""
 rm -f file*
+
+touch file1 file2
+chmod 400 file1 file2
+testing "Move file over unwritable file with no stdin" \
+   "</dev/null mv file2 file1 && [ -e file -a ! -e file2 ] && echo 'yes'" \
+   "yes\n" "" ""
+rm -f file*
diff --git a/toys/posix/cp.c b/toys/posix/cp.c
index d5e92f2..5a55f40 100644
--- a/toys/posix/cp.c
+++ b/toys/posix/cp.c
@@ -382,7 +382,7 @@ void cp_main(void)
         {
           fprintf(stderr, "%s: overwrite '%s'", toys.which->name, TT.destname);
           if (!yesno("", 1)) rc = 0;
-          else unlink(src);
+          else unlink(TT.destname);
         }
       }




Fix mv on overwrite and its prompt.

We need to remove the destination, not the source, to be able to overwrite.

Also, coreutils mv doesn't prompt if it's not talking to a tty. This
change also affects killall, crontab, find, and rm. The rm case at
least is interesting --- coreutils silently *does* do the removal,
whereas this patch would make toybox silently *not* do the removal.
This despite the fact that coreutils rm on a tty defaults to 'n'. So
do we want two different defaults for yesno? Or is this a coreutils
bug? (It certainly seems surprising to me.)

diff --git a/lib/lib.c b/lib/lib.c
index c16cffe..85ea4e1 100644
--- a/lib/lib.c
+++ b/lib/lib.c
@@ -634,6 +634,8 @@ int yesno(char *prompt, int def)
 {
   char buf;

+  if (!isatty(0)) return def;
+
   fprintf(stderr, "%s (%c/%c):", prompt, def ? 'Y' : 'y', def ? 'n' : 'N');
   fflush(stderr);
   while (fread(&buf, 1, 1, stdin)) {
diff --git a/tests/mv.test b/tests/mv.test
index 53fc999..030e9cc 100755
--- a/tests/mv.test
+++ b/tests/mv.test
@@ -96,3 +96,10 @@ touch file1 file2
 testing "Move -n file new_file (exist)" "mv -n file1 file2 &&
    [ -e file1 -a -e file2 ] && echo 'yes'" "yes\n" "" ""
 rm -f file*
+
+touch file1 file2
+chmod 400 file1 file2
+testing "Move file over unwritable file with no stdin" \
+   "</dev/null mv file2 file1 && [ -e file -a ! -e file2 ] && echo 'yes'" \
+   "yes\n" "" ""
+rm -f file*
diff --git a/toys/posix/cp.c b/toys/posix/cp.c
index d5e92f2..c9de14d 100644
--- a/toys/posix/cp.c
+++ b/toys/posix/cp.c
@@ -380,9 +380,10 @@ void cp_main(void)
         if (!stat(TT.destname, &st)
           && ((toys.optflags & FLAG_i) || !(st.st_mode & 0222)))
         {
-          fprintf(stderr, "%s: overwrite '%s'", toys.which->name, TT.destname);
-          if (!yesno("", 1)) rc = 0;
-          else unlink(src);
+          snprintf(toybuf, sizeof(toybuf), "%s: overwrite '%s'",
+                   toys.which->name, TT.destname);
+          if (!yesno(toybuf, 1)) rc = 0;
+          else unlink(TT.destname);
         }
       }



-- 
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.


More information about the Toybox mailing list