{"thread":{"id":"5332","subject":"[PATCH] builtin-mv: readability patch","startedAt":"2006-08-19T14:52:21Z","lastAt":"2006-08-19T14:52:21Z","messageCount":1,"participants":["Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"25616","messageId":"Pine.LNX.4.63.0608191651110.28360@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"5332","inReplyTo":null,"subject":"[PATCH] builtin-mv: readability patch","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-08-19T14:52:21Z","receivedAt":"2006-08-19T14:52:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nThe old version was not liked at all. This is hopefully better. Oh, and it \ngets rid of the goto.\n\nNote that it does not change any functionality.\n\nSigned-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\n---\n builtin-mv.c |  104 +++++++++++++++++++++++++---------------------------------\n 1 files changed, 44 insertions(+), 60 deletions(-)\n\ndiff --git a/builtin-mv.c b/builtin-mv.c\nindex c0c8764..1fdb0c7 100644\n--- a/builtin-mv.c\n+++ b/builtin-mv.c\n@@ -126,48 +126,43 @@ int cmd_mv(int argc, const char **argv, \n \n \t/* Checking */\n \tfor (i = 0; i < count; i++) {\n-\t\tint length;\n+\t\tconst char *src = source[i], *dst = destination[i];\n+\t\tint length, src_is_dir;\n \t\tconst char *bad = NULL;\n \n \t\tif (show_only)\n-\t\t\tprintf(\"Checking rename of '%s' to '%s'\\n\",\n-\t\t\t\tsource[i], destination[i]);\n+\t\t\tprintf(\"Checking rename of '%s' to '%s'\\n\", src, dst);\n \n-\t\tif (lstat(source[i], &st) < 0)\n+\t\tlength = strlen(src);\n+\t\tif (lstat(src, &st) < 0)\n \t\t\tbad = \"bad source\";\n-\n-\t\tif (!bad &&\n-\t\t    (length = strlen(source[i])) >= 0 &&\n-\t\t    !strncmp(destination[i], source[i], length) &&\n-\t\t    (destination[i][length] == 0 || destination[i][length] == '/'))\n+\t\telse if (!strncmp(src, dst, length) &&\n+\t\t\t\t(dst[length] == 0 || dst[length] == '/')) {\n \t\t\tbad = \"can not move directory into itself\";\n-\n-\t\tif (S_ISDIR(st.st_mode)) {\n-\t\t\tconst char *dir = source[i], *dest_dir = destination[i];\n-\t\t\tint first, last, len = strlen(dir);\n-\n-\t\t\tif (lstat(dest_dir, &st) == 0) {\n-\t\t\t\tbad = \"cannot move directory over file\";\n-\t\t\t\tgoto next;\n-\t\t\t}\n+\t\t} else if ((src_is_dir = S_ISDIR(st.st_mode))\n+\t\t\t\t&& lstat(dst, &st) == 0)\n+\t\t\tbad = \"cannot move directory over file\";\n+\t\telse if (src_is_dir) {\n+\t\t\tint first, last;\n \n \t\t\tmodes[i] = WORKING_DIRECTORY;\n \n-\t\t\tfirst = cache_name_pos(source[i], len);\n+\t\t\tfirst = cache_name_pos(src, length);\n \t\t\tif (first >= 0)\n-\t\t\t\tdie (\"Huh? %s/ is in index?\", dir);\n+\t\t\t\tdie (\"Huh? %s/ is in index?\", src);\n \n \t\t\tfirst = -1 - first;\n \t\t\tfor (last = first; last < active_nr; last++) {\n \t\t\t\tconst char *path = active_cache[last]->name;\n-\t\t\t\tif (strncmp(path, dir, len) || path[len] != '/')\n+\t\t\t\tif (strncmp(path, src, length)\n+\t\t\t\t\t\t|| path[length] != '/')\n \t\t\t\t\tbreak;\n \t\t\t}\n \n \t\t\tif (last - first < 1)\n \t\t\t\tbad = \"source directory is empty\";\n-\t\t\telse if (!bad) {\n-\t\t\t\tint j, dst_len = strlen(dest_dir);\n+\t\t\telse {\n+\t\t\t\tint j, dst_len;\n \n \t\t\t\tif (last - first > 0) {\n \t\t\t\t\tsource = realloc(source,\n@@ -181,24 +176,21 @@ int cmd_mv(int argc, const char **argv, \n \t\t\t\t\t\t\t* sizeof(enum update_mode));\n \t\t\t\t}\n \n-\t\t\t\tdest_dir = add_slash(dest_dir);\n+\t\t\t\tdst = add_slash(dst);\n+\t\t\t\tdst_len = strlen(dst) - 1;\n \n \t\t\t\tfor (j = 0; j < last - first; j++) {\n \t\t\t\t\tconst char *path =\n \t\t\t\t\t\tactive_cache[first + j]->name;\n \t\t\t\t\tsource[count + j] = path;\n \t\t\t\t\tdestination[count + j] =\n-\t\t\t\t\t\tprefix_path(dest_dir, dst_len,\n-\t\t\t\t\t\t\tpath + len);\n+\t\t\t\t\t\tprefix_path(dst, dst_len,\n+\t\t\t\t\t\t\tpath + length);\n \t\t\t\t\tmodes[count + j] = INDEX;\n \t\t\t\t}\n \t\t\t\tcount += last - first;\n \t\t\t}\n-\n-\t\t\tgoto next;\n-\t\t}\n-\n-\t\tif (!bad && lstat(destination[i], &st) == 0) {\n+\t\t} else if (lstat(dst, &st) == 0) {\n \t\t\tbad = \"destination exists\";\n \t\t\tif (force) {\n \t\t\t\t/*\n@@ -210,24 +202,17 @@ int cmd_mv(int argc, const char **argv, \n \t\t\t\t\t\t\t\" will overwrite!\\n\",\n \t\t\t\t\t\t\tbad);\n \t\t\t\t\tbad = NULL;\n-\t\t\t\t\tpath_list_insert(destination[i],\n-\t\t\t\t\t\t\t&overwritten);\n+\t\t\t\t\tpath_list_insert(dst, &overwritten);\n \t\t\t\t} else\n \t\t\t\t\tbad = \"Cannot overwrite\";\n \t\t\t}\n-\t\t}\n-\n-\t\tif (!bad && cache_name_pos(source[i], strlen(source[i])) < 0)\n+\t\t} else if (cache_name_pos(src, length) < 0)\n \t\t\tbad = \"not under version control\";\n+\t\telse if (path_list_has_path(&src_for_dst, dst))\n+\t\t\tbad = \"multiple sources for the same target\";\n+\t\telse\n+\t\t\tpath_list_insert(dst, &src_for_dst);\n \n-\t\tif (!bad) {\n-\t\t\tif (path_list_has_path(&src_for_dst, destination[i]))\n-\t\t\t\tbad = \"multiple sources for the same target\";\n-\t\t\telse\n-\t\t\t\tpath_list_insert(destination[i], &src_for_dst);\n-\t\t}\n-\n-next:\n \t\tif (bad) {\n \t\t\tif (ignore_errors) {\n \t\t\t\tif (--count > 0) {\n@@ -239,33 +224,32 @@ next:\n \t\t\t\t}\n \t\t\t} else\n \t\t\t\tdie (\"%s, source=%s, destination=%s\",\n-\t\t\t\t     bad, source[i], destination[i]);\n+\t\t\t\t     bad, src, dst);\n \t\t}\n \t}\n \n \tfor (i = 0; i < count; i++) {\n+\t\tconst char *src = source[i], *dst = destination[i];\n+\t\tenum update_mode mode = modes[i];\n \t\tif (show_only || verbose)\n-\t\t\tprintf(\"Renaming %s to %s\\n\",\n-\t\t\t       source[i], destination[i]);\n-\t\tif (!show_only && modes[i] != INDEX &&\n-\t\t    rename(source[i], destination[i]) < 0 &&\n-\t\t    !ignore_errors)\n-\t\t\tdie (\"renaming %s failed: %s\",\n-\t\t\t     source[i], strerror(errno));\n-\n-\t\tif (modes[i] == WORKING_DIRECTORY)\n+\t\t\tprintf(\"Renaming %s to %s\\n\", src, dst);\n+\t\tif (!show_only && mode != INDEX &&\n+\t\t\t\trename(src, dst) < 0 && !ignore_errors)\n+\t\t\tdie (\"renaming %s failed: %s\", src, strerror(errno));\n+\n+\t\tif (mode == WORKING_DIRECTORY)\n \t\t\tcontinue;\n \n-\t\tif (cache_name_pos(source[i], strlen(source[i])) >= 0) {\n-\t\t\tpath_list_insert(source[i], &deleted);\n+\t\tif (cache_name_pos(src, strlen(src)) >= 0) {\n+\t\t\tpath_list_insert(src, &deleted);\n \n \t\t\t/* destination can be a directory with 1 file inside */\n-\t\t\tif (path_list_has_path(&overwritten, destination[i]))\n-\t\t\t\tpath_list_insert(destination[i], &changed);\n+\t\t\tif (path_list_has_path(&overwritten, dst))\n+\t\t\t\tpath_list_insert(dst, &changed);\n \t\t\telse\n-\t\t\t\tpath_list_insert(destination[i], &added);\n+\t\t\t\tpath_list_insert(dst, &added);\n \t\t} else\n-\t\t\tpath_list_insert(destination[i], &added);\n+\t\t\tpath_list_insert(dst, &added);\n \t}\n \n         if (show_only) {\n-- \n1.4.2.gd3f3-dirty\n"}]}