On Mon, Aug 31, 2015 at 12:20 PM, enh <[email protected]> wrote: > On Sun, Aug 30, 2015 at 9:15 PM, Rob Landley <[email protected]> wrote: >> On 08/28/2015 08:02 PM, enh wrote: >>> 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)) { >> >> Reasonable. >> >>> 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* >> >> Already checked in as part of earlier commit. >> >>> 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); >>> } >>> } >> >> The reason I didn't do that (and the reason yesno() was initially >> designed to let you print the start of the message yourself) is the name >> isn't guaranteed to fit into toybuf. You're trimming to the >> sizeof(toybuf) in the snprintf (I just did five minutes of digging >> because the snprintf() man page is very unclear about whether the null >> terminator is guaranteed to be written, but luckily the posix page on it >> _is_ clear and it is in the nonzero size case). >> >> That said, truncating the filename is breakage. (Long path only shows >> path, no idea what file we're referring to.) I'm intentionally doing an >> openat() based approach to support arbitrarily deep filenames because >> you can create them after the fact with directory mv so you simply can't >> avoid 'em, and we just gotta cope. >> >> That said have an xmprintf() in lib/xwrap.c for exactly this purpose. >> It's sad to churn malloc/free but the -i case is very much _not_ a >> hotpath, so... (Or I could yank -i if !isatty(0) in main. :) > > good point. an alternative would be to make yesno take a format > string. i can send you a patch to do that if you'd like to go that > route. (it would mean that the obvious code would then be correct even > in long-path cases.)
i've sent a patch to make yesno printf-like because fixed-length limits suck and existing uses of yesno often end up either formatting into toybuf or writing to stderr themselves. >> But before I check any of that in... I have half a human_readable() >> change in my lib/lib.c. Right, need to clean up all this half-finished >> stuff in my workspace... >> >> Rob > > > > -- > Elliott Hughes - http://who/enh - http://jessies.org/~enh/ > Android native code/tools questions? Mail me/drop by/add me as a reviewer. -- Elliott Hughes - http://who/enh - http://jessies.org/~enh/ Android native code/tools questions? Mail me/drop by/add me as a reviewer. _______________________________________________ Toybox mailing list [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
