cp,mv: simpler arg[cv] handling -> smallish code savings

3 files changed, 39 insertions, 34 deletions

diff --git a/coreutils/cp.c b/coreutils/cp.c
index 884fbf70f..5b575819c 100644
--- a/coreutils/cp.c
+++ b/coreutils/cp.c
@@ -40,12 +40,16 @@ int cp_main(int argc, char **argv)
                 OPT_L = 1 << (sizeof(FILEUTILS_CP_OPTSTR)+3),
         };
 
+        // Need at least two arguments
         // Soft- and hardlinking don't mix
         // -P and -d are the same (-P is POSIX, -d is GNU)
         // -r and -R are the same
         // -a = -pdR
-        opt_complementary = "l--s:s--l:Pd:rR:apdR";
+        opt_complementary = "-2:l--s:s--l:Pd:rR:apdR";
         flags = getopt32(argv, FILEUTILS_CP_OPTSTR "arPHL");
+        argc -= optind;
+        argv += optind;
+        flags ^= FILEUTILS_DEREFERENCE;         /* The sense of this flag was reversed. */
         /* Default behavior of cp is to dereference, so we don't have to do
          * anything special when we are given -L.
          * The behavior of -H is *almost* like -L, but not quite, so let's
@@ -60,19 +64,12 @@ int cp_main(int argc, char **argv)
         }
 #endif
 
-        flags ^= FILEUTILS_DEREFERENCE;         /* The sense of this flag was reversed. */
-
-        if (optind + 2 > argc) {
-                bb_show_usage();
-        }
-
         last = argv[argc - 1];
-        argv += optind;
-
         /* If there are only two arguments and...  */
-        if (optind + 2 == argc) {
+        if (argc == 2) {
                 s_flags = cp_mv_stat2(*argv, &source_stat,
                                       (flags & FILEUTILS_DEREFERENCE) ? stat : lstat);
+                /* TODO: does coreutils cp exit? "cp BAD GOOD dir"... */
                 if (s_flags < 0)
                         return EXIT_FAILURE;
                 d_flags = cp_mv_stat(last, &dest_stat);
@@ -85,19 +82,24 @@ int cp_main(int argc, char **argv)
                         ((flags & FILEUTILS_RECUR) && (s_flags & 2) && !d_flags)
                 ) {
                         /* ...do a simple copy.  */
-                        dest = xstrdup(last);
-                        goto DO_COPY; /* Note: optind+2==argc implies argv[1]==last below. */
+                        dest = last;
+                        goto DO_COPY; /* NB: argc==2 -> *++argv==last */
                 }
         }
 
-        do {
+        while (1) {
                 dest = concat_path_file(last, bb_get_last_path_component(*argv));
  DO_COPY:
                 if (copy_file(*argv, dest, flags) < 0) {
                         status = 1;
                 }
+                if (*++argv == last) {
+                        /* possibly leaking dest... */
+                        break;
+                }
                 free((void*)dest);
-        } while (*++argv != last);
+        }
 
+        /* Exit. We are NOEXEC, not NOFORK. We do exit at the end of main() */
         return status;
 }
diff --git a/coreutils/install.c b/coreutils/install.c
index cf62a0022..d087306d6 100644
--- a/coreutils/install.c
+++ b/coreutils/install.c
@@ -123,10 +123,12 @@ int install_main(int argc, char **argv)
                 copy_flags |= FILEUTILS_PRESERVE_STATUS;
         }
         mode = 0666;
-        if (flags & OPT_MODE) bb_parse_mode(mode_str, &mode);
+        if (flags & OPT_MODE)
+                bb_parse_mode(mode_str, &mode);
         uid = (flags & OPT_OWNER) ? get_ug_id(uid_str, xuname2uid) : getuid();
         gid = (flags & OPT_GROUP) ? get_ug_id(gid_str, xgroup2gid) : getgid();
-        if (flags & (OPT_OWNER|OPT_GROUP)) umask(0);
+        if (flags & (OPT_OWNER|OPT_GROUP))
+                umask(0);
 
         /* Create directories
          * don't use bb_make_directory() as it can't change uid or gid
diff --git a/coreutils/mv.c b/coreutils/mv.c
index 553bb6ecb..1d2977060 100644
--- a/coreutils/mv.c
+++ b/coreutils/mv.c
@@ -47,16 +47,15 @@ int mv_main(int argc, char **argv)
 #if ENABLE_FEATURE_MV_LONG_OPTIONS
         applet_long_options = mv_longopts;
 #endif
-        opt_complementary = "f-i:i-f";
+        // Need at least two arguments
+        // -f unsets -i, -i unsets -f
+        opt_complementary = "-2:f-i:i-f";
         flags = getopt32(argv, "fi");
-        if (optind + 2 > argc) {
-                bb_show_usage();
-        }
-
-        last = argv[argc - 1];
+        argc -= optind;
         argv += optind;
+        last = argv[argc - 1];
 
-        if (optind + 2 == argc) {
+        if (argc == 2) {
                 dest_exists = cp_mv_stat(last, &dest_stat);
                 if (dest_exists < 0) {
                         return 1;
@@ -75,11 +74,11 @@ int mv_main(int argc, char **argv)
                         goto RET_1;
                 }
 
-DO_MOVE:
-
-                if (dest_exists && !(flags & OPT_FILEUTILS_FORCE) &&
-                        ((access(dest, W_OK) < 0 && isatty(0)) ||
-                        (flags & OPT_FILEUTILS_INTERACTIVE))) {
+ DO_MOVE:
+                if (dest_exists && !(flags & OPT_FILEUTILS_FORCE)
+                 && ((access(dest, W_OK) < 0 && isatty(0))
+                    || (flags & OPT_FILEUTILS_INTERACTIVE))
+                ) {
                         if (fprintf(stderr, "mv: overwrite '%s'? ", dest) < 0) {
                                 goto RET_1;     /* Ouch! fprintf failed! */
                         }
@@ -91,8 +90,9 @@ DO_MOVE:
                         struct stat source_stat;
                         int source_exists;
 
-                        if (errno != EXDEV ||
-                                (source_exists = cp_mv_stat(*argv, &source_stat)) < 1) {
+                        if (errno != EXDEV
+                         || (source_exists = cp_mv_stat(*argv, &source_stat)) < 1
+                        ) {
                                 bb_perror_msg("cannot rename '%s'", *argv);
                         } else {
                                 if (dest_exists) {
@@ -116,15 +116,16 @@ DO_MOVE:
 #if ENABLE_SELINUX
                                 copy_flag |= FILEUTILS_PRESERVE_SECURITY_CONTEXT;
 #endif
-                                if ((copy_file(*argv, dest, copy_flag) >= 0) &&
-                                        (remove_file(*argv, FILEUTILS_RECUR | FILEUTILS_FORCE) >= 0)) {
+                                if ((copy_file(*argv, dest, copy_flag) >= 0)
+                                 && (remove_file(*argv, FILEUTILS_RECUR | FILEUTILS_FORCE) >= 0)
+                                ) {
                                         goto RET_0;
                                 }
                         }
-RET_1:
+ RET_1:
                         status = 1;
                 }
-RET_0:
+ RET_0:
                 if (dest != last) {
                         free((void *) dest);
                 }