builtin-mv: readability patch

The old version was not liked at all. This is hopefully better. Oh, and it
gets rid of the goto.

Note that it does not change any functionality.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Junio C Hamano <junkio@cox.net>

diff --git a/builtin-mv.c b/builtin-mv.c
index e3bc7a86bb0732f5db760151088c73c87f88e637..ff882bec474c59c9cb4bb49d723cf81febb204ad 100644 (file)
--- a/builtin-mv.c
+++ b/builtin-mv.c
@@ -129,48 +129,43 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 
        /* Checking */
        for (i = 0; i < count; i++) {
 
        /* Checking */
        for (i = 0; i < count; i++) {

-               int length;
+               const char *src = source[i], *dst = destination[i];
+               int length, src_is_dir;

                const char *bad = NULL;
 
                if (show_only)
                const char *bad = NULL;
 
                if (show_only)

-                       printf("Checking rename of '%s' to '%s'\n",
-                               source[i], destination[i]);
+                       printf("Checking rename of '%s' to '%s'\n", src, dst);

 
 

-               if (lstat(source[i], &st) < 0)
+               length = strlen(src);
+               if (lstat(src, &st) < 0)

                        bad = "bad source";
                        bad = "bad source";

-
-               if (!bad &&
-                   (length = strlen(source[i])) >= 0 &&
-                   !strncmp(destination[i], source[i], length) &&
-                   (destination[i][length] == 0 || destination[i][length] == '/'))
+               else if (!strncmp(src, dst, length) &&
+                               (dst[length] == 0 || dst[length] == '/')) {

                        bad = "can not move directory into itself";
                        bad = "can not move directory into itself";

-
-               if (S_ISDIR(st.st_mode)) {
-                       const char *dir = source[i], *dest_dir = destination[i];
-                       int first, last, len = strlen(dir);
-
-                       if (lstat(dest_dir, &st) == 0) {
-                               bad = "cannot move directory over file";
-                               goto next;
-                       }
+               } else if ((src_is_dir = S_ISDIR(st.st_mode))
+                               && lstat(dst, &st) == 0)
+                       bad = "cannot move directory over file";
+               else if (src_is_dir) {
+                       int first, last;

 
                        modes[i] = WORKING_DIRECTORY;
 
 
                        modes[i] = WORKING_DIRECTORY;
 

-                       first = cache_name_pos(source[i], len);
+                       first = cache_name_pos(src, length);

                        if (first >= 0)
                        if (first >= 0)

-                               die ("Huh? %s/ is in index?", dir);
+                               die ("Huh? %s/ is in index?", src);

 
                        first = -1 - first;
                        for (last = first; last < active_nr; last++) {
                                const char *path = active_cache[last]->name;
 
                        first = -1 - first;
                        for (last = first; last < active_nr; last++) {
                                const char *path = active_cache[last]->name;

-                               if (strncmp(path, dir, len) || path[len] != '/')
+                               if (strncmp(path, src, length)
+                                               || path[length] != '/')

                                        break;
                        }
 
                        if (last - first < 1)
                                bad = "source directory is empty";
                                        break;
                        }
 
                        if (last - first < 1)
                                bad = "source directory is empty";

-                       else if (!bad) {
-                               int j, dst_len = strlen(dest_dir);
+                       else {
+                               int j, dst_len;

 
                                if (last - first > 0) {
                                        source = realloc(source,
 
                                if (last - first > 0) {
                                        source = realloc(source,


@@ -184,24 +179,21 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
                                                        * sizeof(enum update_mode));
                                }
 
                                                        * sizeof(enum update_mode));
                                }
 

-                               dest_dir = add_slash(dest_dir);
+                               dst = add_slash(dst);
+                               dst_len = strlen(dst) - 1;

 
                                for (j = 0; j < last - first; j++) {
                                        const char *path =
                                                active_cache[first + j]->name;
                                        source[count + j] = path;
                                        destination[count + j] =
 
                                for (j = 0; j < last - first; j++) {
                                        const char *path =
                                                active_cache[first + j]->name;
                                        source[count + j] = path;
                                        destination[count + j] =

-                                               prefix_path(dest_dir, dst_len,
-                                                       path + len);
+                                               prefix_path(dst, dst_len,
+                                                       path + length);

                                        modes[count + j] = INDEX;
                                }
                                count += last - first;
                        }
                                        modes[count + j] = INDEX;
                                }
                                count += last - first;
                        }

-
-                       goto next;
-               }
-
-               if (!bad && lstat(destination[i], &st) == 0) {
+               } else if (lstat(dst, &st) == 0) {

                        bad = "destination exists";
                        if (force) {
                                /*
                        bad = "destination exists";
                        if (force) {
                                /*


@@ -213,24 +205,17 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
                                                        " will overwrite!\n",
                                                        bad);
                                        bad = NULL;
                                                        " will overwrite!\n",
                                                        bad);
                                        bad = NULL;

-                                       path_list_insert(destination[i],
-                                                       &overwritten);
+                                       path_list_insert(dst, &overwritten);

                                } else
                                        bad = "Cannot overwrite";
                        }
                                } else
                                        bad = "Cannot overwrite";
                        }

-               }
-
-               if (!bad && cache_name_pos(source[i], strlen(source[i])) < 0)
+               } else if (cache_name_pos(src, length) < 0)

                        bad = "not under version control";
                        bad = "not under version control";

+               else if (path_list_has_path(&src_for_dst, dst))
+                       bad = "multiple sources for the same target";
+               else
+                       path_list_insert(dst, &src_for_dst);

 
 

-               if (!bad) {
-                       if (path_list_has_path(&src_for_dst, destination[i]))
-                               bad = "multiple sources for the same target";
-                       else
-                               path_list_insert(destination[i], &src_for_dst);
-               }
-
-next:

                if (bad) {
                        if (ignore_errors) {
                                if (--count > 0) {
                if (bad) {
                        if (ignore_errors) {
                                if (--count > 0) {


@@ -242,33 +227,32 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
                                }
                        } else
                                die ("%s, source=%s, destination=%s",
                                }
                        } else
                                die ("%s, source=%s, destination=%s",

-                                    bad, source[i], destination[i]);
+                                    bad, src, dst);

                }
        }
 
        for (i = 0; i < count; i++) {
                }
        }
 
        for (i = 0; i < count; i++) {

+               const char *src = source[i], *dst = destination[i];
+               enum update_mode mode = modes[i];

                if (show_only || verbose)
                if (show_only || verbose)

-                       printf("Renaming %s to %s\n",
-                              source[i], destination[i]);
-               if (!show_only && modes[i] != INDEX &&
-                   rename(source[i], destination[i]) < 0 &&
-                   !ignore_errors)
-                       die ("renaming %s failed: %s",
-                            source[i], strerror(errno));
-
-               if (modes[i] == WORKING_DIRECTORY)
+                       printf("Renaming %s to %s\n", src, dst);
+               if (!show_only && mode != INDEX &&
+                               rename(src, dst) < 0 && !ignore_errors)
+                       die ("renaming %s failed: %s", src, strerror(errno));
+
+               if (mode == WORKING_DIRECTORY)

                        continue;
 
                        continue;
 

-               if (cache_name_pos(source[i], strlen(source[i])) >= 0) {
-                       path_list_insert(source[i], &deleted);
+               if (cache_name_pos(src, strlen(src)) >= 0) {
+                       path_list_insert(src, &deleted);

 
                        /* destination can be a directory with 1 file inside */
 
                        /* destination can be a directory with 1 file inside */

-                       if (path_list_has_path(&overwritten, destination[i]))
-                               path_list_insert(destination[i], &changed);
+                       if (path_list_has_path(&overwritten, dst))
+                               path_list_insert(dst, &changed);

                        else
                        else

-                               path_list_insert(destination[i], &added);
+                               path_list_insert(dst, &added);

                } else
                } else

-                       path_list_insert(destination[i], &added);
+                       path_list_insert(dst, &added);

        }
 
         if (show_only) {
        }
 
         if (show_only) {