{"thread":{"id":"4997","subject":"[PATCH 2/2] builtin git-mv: support moving directories","startedAt":"2006-07-26T17:47:54Z","lastAt":"2006-08-08T22:36:35Z","messageCount":5,"participants":["Johannes Schindelin","Junio C Hamano","Juergen Ruehle"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"24143","messageId":"Pine.LNX.4.63.0607261941210.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4997","inReplyTo":null,"subject":"[PATCH 2/2] builtin git-mv: support moving directories","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-07-26T17:47:54Z","receivedAt":"2006-07-26T17:47:54Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nThis fixes the builtin mv for the test which Josef provided, and also\nfixes moving directories into existing directories, as noted by Jon Smirl.\nIn case the destination exists, fail early (this cannot be overridden\nby -f).\n\nSigned-off-by: Johannes Schindelin \n---\n\n\tJosef, this passes your test.\n\n\tJon, if I understood your problem, then this fixes that, too.\n\n\tEveryone: it would be cleaner to use \"goto next\" everywhere, \n\tinstead of all these \"if (!bad && ...\" constructs, but I'll leave \n\tthat for other people ;-)\n\n builtin-mv.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---\n t/t7001-mv.sh | 4 +++\n 2 files changed, 86 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin-mv.c b/builtin-mv.c\nindex 593ff9e..e660a9f 100644\n--- a/builtin-mv.c\n+++ b/builtin-mv.c\n@@ -42,6 +42,18 @@ static void show_list(const char *label,\n \t}\n }\n \n+static const char *add_slash(const char *path)\n+{\n+\tint len = strlen(path);\n+\tif (path[len - 1] != '/') {\n+\t\tchar *with_slash = xmalloc(len + 2);\n+\t\tmemcpy(with_slash, path, len);\n+\t\tstrcat(with_slash + len, \"/\");\n+\t\treturn with_slash;\n+\t}\n+\treturn path;\n+}\n+\n static struct lock_file lock_file;\n \n int cmd_mv(int argc, const char **argv, char **envp)\n@@ -50,6 +62,7 @@ int cmd_mv(int argc, const char **argv, \n \tint verbose = 0, show_only = 0, force = 0, ignore_errors = 0;\n \tconst char *prefix = setup_git_directory();\n \tconst char **source, **destination, **dest_path;\n+\tenum update_mode { BOTH = 0, WORKING_DIRECTORY, INDEX } *modes;\n \tstruct stat st;\n \tstruct path_list overwritten = {NULL, 0, 0, 0};\n \tstruct path_list src_for_dst = {NULL, 0, 0, 0};\n@@ -94,11 +107,14 @@ int cmd_mv(int argc, const char **argv, \n \t\tusage(builtin_mv_usage);\n \n \tsource = copy_pathspec(prefix, argv + i, count, 0);\n+ \tmodes = xcalloc(count, sizeof(enum update_mode));\n \tdest_path = copy_pathspec(prefix, argv + argc - 1, 1, 0);\n \n-\tif (!lstat(dest_path[0], &st) && S_ISDIR(st.st_mode))\n+\tif (!lstat(dest_path[0], &st) &&\n+\t\t\tS_ISDIR(st.st_mode)) {\n+ \t\tdest_path[0] = add_slash(dest_path[0]);\n \t\tdestination = copy_pathspec(dest_path[0], argv + i, count, 1);\n-\telse {\n+\t} else {\n \t\tif (count != 1)\n \t\t\tusage(builtin_mv_usage);\n \t\tdestination = dest_path;\n@@ -114,7 +130,64 @@ int cmd_mv(int argc, const char **argv, \n \n \t\tif (lstat(source[i], &st) < 0)\n \t\t\tbad = \"bad source\";\n-\t\telse if (lstat(destination[i], &st) == 0) {\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+\n+\t\t\tmodes[i] = WORKING_DIRECTORY;\n+\n+\t\t\tfirst = cache_name_pos(source[i], len);\n+\t\t\tif (first >= 0)\n+\t\t\t\tdie (\"Huh? %s/ is in index?\", dir);\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\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+\n+\t\t\t\tif (last - first > 0) {\n+\t\t\t\t\tsource = realloc(source,\n+\t\t\t\t\t\t\t(count + last - first)\n+\t\t\t\t\t\t\t* sizeof(char *));\n+\t\t\t\t\tdestination = realloc(destination,\n+\t\t\t\t\t\t\t(count + last - first)\n+\t\t\t\t\t\t\t* sizeof(char *));\n+\t\t\t\t\tmodes = realloc(modes,\n+\t\t\t\t\t\t\t(count + last - first)\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+\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\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\tbad = \"destination exists\";\n \t\t\tif (force) {\n \t\t\t\t/*\n@@ -147,6 +220,7 @@ int cmd_mv(int argc, const char **argv, \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@@ -157,7 +231,7 @@ int cmd_mv(int argc, const char **argv, \n \t\t\t\t\t\t(count - i) * sizeof(char *));\n \t\t\t\t}\n \t\t\t} else\n-\t\t\t\tdie (\"Error: %s, source=%s, destination=%s\",\n+\t\t\t\tdie (\"%s, source=%s, destination=%s\",\n \t\t\t\t bad, source[i], destination[i]);\n \t\t}\n \t}\n@@ -166,12 +240,15 @@ int cmd_mv(int argc, const char **argv, \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 &&\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\tcontinue;\n+ \n \t\tif (cache_name_pos(source[i], strlen(source[i])) >= 0) {\n \t\t\tpath_list_insert(source[i], &deleted);\n \ndiff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\nindex 322eaad..900ca93 100755\n--- a/t/t7001-mv.sh\n+++ b/t/t7001-mv.sh\n@@ -74,4 +74,8 @@ test_expect_success \\\n git-diff-tree -r -M --name-status HEAD^ HEAD | \\\n grep -E \"^R100.+path2/README.+path1/path2/README\"'\n \n+test_expect_failure \\\n+ 'do not move directory over existing directory' \\\n+ 'mkdir path0 && mkdir path0/path2 && git-mv path2 path0'\n+\n test_done\n-- \n1.4.2.rc2.g96f2-dirty\n"},{"id":"24160","messageId":"7vbqrcqgsc.fsf@assigned-by-dhcp.cox.net","threadId":"4997","inReplyTo":"Pine.LNX.4.63.0607261941210.29667@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH 2/2] builtin git-mv: support moving directories","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-07-26T19:58:59Z","receivedAt":"2006-07-26T19:58:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks. Will apply.\n"},{"id":"24988","messageId":"17624.55600.423000.61479@lapjr.intranet.kiel.bmiag.de","threadId":"4997","inReplyTo":"Pine.LNX.4.63.0607261941210.29667@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH 2/2] builtin git-mv: support moving directories","fromName":"Juergen Ruehle","fromEmail":"j.ruehle@bmiag.de","sentAt":"2006-08-08T18:34:24Z","receivedAt":"2006-08-08T18:34:24Z","isPatch":true,"sender":{"key":"j.ruehle@bmiag.de","avatar":null},"body":"This might have been fixed already (my tree is a couple of days old),\nbut the strcat fails for me, because the alloced memory is not\ncleared.\n\nJohannes Schindelin writes:\n > +static const char *add_slash(const char *path)\n > +{\n > +\tint len = strlen(path);\n > +\tif (path[len - 1] != '/') {\n > +\t\tchar *with_slash = xmalloc(len + 2);\n > +\t\tmemcpy(with_slash, path, len);\n > +\t\tstrcat(with_slash + len, \"/\");\n > +\t\treturn with_slash;\n > +\t}\n > +\treturn path;\n > +}\n\nperhaps morph the strcat into a memcopy or append the slash and the\nNUL manually?\n\n jr\n"},{"id":"24991","messageId":"7virl3vwck.fsf@assigned-by-dhcp.cox.net","threadId":"4997","inReplyTo":"17624.55600.423000.61479@lapjr.intranet.kiel.bmiag.de","subject":"Re: [PATCH 2/2] builtin git-mv: support moving directories","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-08-08T19:52:27Z","receivedAt":"2006-08-08T19:52:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Juergen Ruehle writes:\n\n> This might have been fixed already (my tree is a couple of days old),\n> but the strcat fails for me, because the alloced memory is not\n> cleared.\n>\n> Johannes Schindelin writes:\n> > +static const char *add_slash(const char *path)\n> > +{\n> > +\tint len = strlen(path);\n> > +\tif (path[len - 1] != '/') {\n> > +\t\tchar *with_slash = xmalloc(len + 2);\n> > +\t\tmemcpy(with_slash, path, len);\n> > +\t\tstrcat(with_slash + len, \"/\");\n> > +\t\treturn with_slash;\n> > +\t}\n> > +\treturn path;\n> > +}\n>\n> perhaps morph the strcat into a memcopy or append the slash and the\n> NUL manually?\n\nThanks, well spotted. A textually minimum change would be to\ndo:\n\n- \t\tmemcpy(with_slash, path, len);\n+ \t\tmemcpy(with_slash, path, len + 1);\n\nbut that would touch the end of the string twice, so manually\nterminating the string with '/' NUL would be appropriate.\n\nI will apply a patch I've been privately using from time to time\nto catch something like this to \"master\".\n\n-- >8 --\n[PATCH] debugging: XMALLOC_POISON\n\nCompile with -DXMALLOC_POISON=1 to catch errors from using uninitialized\nmemory returned by xmalloc.\n\nSigned-off-by: Junio C Hamano \n---\n git-compat-util.h | 3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 93f5580..3bcf5b1 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -91,6 +91,9 @@ static inline void *xmalloc(size_t size)\n \t\tret = malloc(1);\n \tif (!ret)\n \t\tdie(\"Out of memory, malloc failed\");\n+#ifdef XMALLOC_POISON\n+\tmemset(ret, 0xA5, size);\n+#endif\n \treturn ret;\n }\n \n-- \n1.4.2.rc3.g45c5\n"},{"id":"25003","messageId":"Pine.LNX.4.63.0608090031390.1800@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4997","inReplyTo":"17624.55600.423000.61479@lapjr.intranet.kiel.bmiag.de","subject":"[PATCH] mv: strcat -> strcpy to account for uninitialized memory","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-08-08T22:36:35Z","receivedAt":"2006-08-08T22:36:35Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nAt one stage, the code assumed erroneously that the memory was initialized \nto zero.\n\nSigned-off-by: Johannes Schindelin \n---\n\n\tOn Tue, 8 Aug 2006, Juergen Ruehle wrote:\n\n\t> This might have been fixed already (my tree is a couple of days old),\n\t> but the strcat fails for me, because the alloced memory is not\n\t> cleared.\n\t> \n\t> Johannes Schindelin writes:\n\t> > +static const char *add_slash(const char *path)\n\t> > +{\n\t> > +\tint len = strlen(path);\n\t> > +\tif (path[len - 1] != '/') {\n\t> > +\t\tchar *with_slash = xmalloc(len + 2);\n\t> > +\t\tmemcpy(with_slash, path, len);\n\t> > +\t\tstrcat(with_slash + len, \"/\");\n\n\tOops. That strcat should be a strcpy, obviously.\n\n builtin-mv.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-mv.c b/builtin-mv.c\nindex e47942c..5f57870 100644\n--- a/builtin-mv.c\n+++ b/builtin-mv.c\n@@ -48,7 +48,7 @@ static const char *add_slash(const char \n \tif (path[len - 1] != '/') {\n \t\tchar *with_slash = xmalloc(len + 2);\n \t\tmemcpy(with_slash, path, len);\n-\t\tstrcat(with_slash + len, \"/\");\n+\t\tstrcpy(with_slash + len, \"/\");\n \t\treturn with_slash;\n \t}\n \treturn path;\n-- \n1.4.2.rc3.g6b27-dirty\n"}]}