git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2] mv: prevent mismatched data when ignoring errors.

From
brian m. carlson <sandals@crustytoothpaste.net>
Date
Mar 15, 2014, 18:56 UTC
Message-ID
<1394909812-92472-1-git-send-email-sandals@crustytoothpaste.net>
In-Reply-To
<1394306499-50871-1-git-send-email-sandals@crustytoothpaste.net>

We shrink the source and destination arrays, but not the modes or submodule_gitfile arrays, resulting in potentially mismatched data. Shrink all the arrays at the same time to prevent this. Add tests to ensure the problem does not recur.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---

I attempted to come up with a second patch that would refactor out the four different arrays into one array of struct, as Jeff suggested, but it became very ugly very quickly. So this patch simply fixes the problem and adds tests.

 builtin/mv.c  |  5 +++++
 t/t7001-mv.sh | 13 ++++++++++++-
 2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/builtin/mv.c b/builtin/mv.c
index f99c91e..09bbc63 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -230,6 +230,11 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 					memmove(destination + i,
 						destination + i + 1,
 						(argc - i) * sizeof(char *));
+					memmove(modes + i, modes + i + 1,
+						(argc - i) * sizeof(enum update_mode));
+					memmove(submodule_gitfile + i,
+						submodule_gitfile + i + 1,
+						(argc - i) * sizeof(char *));
 					i--;
 				}
 			} else
diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index e3c8c2c..215d43d 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -294,7 +294,8 @@ test_expect_success 'setup submodule' '
 	git submodule add ./. sub &&
 	echo content >file &&
 	git add file &&
-	git commit -m "added sub and file"
+	git commit -m "added sub and file" &&
+	git branch submodule
 '
 
 test_expect_success 'git mv cannot move a submodule in a file' '
@@ -463,4 +464,14 @@ test_expect_success 'checking out a commit before submodule moved needs manual u
 	! test -s actual
 '
 
+test_expect_success 'mv -k does not accidentally destroy submodules' '
+	git checkout submodule &&
+	mkdir dummy dest &&
+	git mv -k dummy sub dest &&
+	git status --porcelain >actual &&
+	grep "^R  sub -> dest/sub" actual &&
+	git reset --hard &&
+	git checkout .
+'
+
 test_done
-- 
1.9.0.1010.g6633b85.dirty
Previous: Junio C HamanoNext: Jeff King
Message 20 of 21 in “git 1.9.0 segfault”
  1. Guillaume GelinMar 8, 2014
  2. brian m. carlsonMar 8, 2014
  3. John KeepingMar 8, 2014
  4. builtin/mv: fix out of bounds writeJohn Keeping, Mar 8, 2014
  5. brian m. carlsonMar 8, 2014
  6. builtin/mv: fix out of bounds writeJohn Keeping, Mar 8, 2014
  7. mv: prevent mismatched data when ignoring errors.brian m. carlson, Mar 8, 2014
  8. Jeff KingMar 11, 2014
  9. brian m. carlsonMar 11, 2014
  10. Junio C HamanoMar 11, 2014
  11. brian m. carlsonMar 12, 2014
  12. Thomas RastMar 15, 2014
  13. Jeff KingMar 16, 2014
  14. Junio C HamanoMar 16, 2014
  15. Junio C HamanoMar 17, 2014
  16. Michael HaggertyMar 17, 2014
  17. Eric SunshineMar 17, 2014
  18. Jeff KingMar 17, 2014
  19. Junio C HamanoMar 18, 2014
  20. mv: prevent mismatched data when ignoring errors.brian m. carlson, Mar 15, 2014
  21. Jeff KingMar 16, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.