Merge branch 'jk/leakfixes'

Memory leaks in "git mv" has been plugged.

* jk/leakfixes:
  mv: replace src_dir with a strvec
  mv: factor out empty src_dir removal
  mv: move src_dir cleanup to end of cmd_mv()
  t-strvec: mark variable-arg helper with LAST_ARG_MUST_BE_NULL
  t-strvec: use va_end() to match va_start()

builtin/mv.c

@@ -156,6 +156,28 @@ free_return:
 	return ret;
 }
 
+static void remove_empty_src_dirs(const char **src_dir, size_t src_dir_nr)
+{
+	size_t i;
+	struct strbuf a_src_dir = STRBUF_INIT;
+
+	for (i = 0; i < src_dir_nr; i++) {
+		int dummy;
+		strbuf_addstr(&a_src_dir, src_dir[i]);
+		/*
+		 * if entries under a_src_dir are all moved away,
+		 * recursively remove a_src_dir to cleanup
+		 */
+		if (index_range_of_same_dir(a_src_dir.buf, a_src_dir.len,
+					    &dummy, &dummy) < 1) {
+			remove_dir_recursively(&a_src_dir, 0);
+		}
+		strbuf_reset(&a_src_dir);
+	}
+
+	strbuf_release(&a_src_dir);
+}
+
 int cmd_mv(int argc, const char **argv, const char *prefix)
 {
 	int i, flags, gitmodules_modified = 0;
@@ -175,9 +197,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 	struct strvec submodule_gitfiles_to_free = STRVEC_INIT;
 	const char **submodule_gitfiles;
 	char *dst_w_slash = NULL;
-	const char **src_dir = NULL;
+	struct strvec src_dir = STRVEC_INIT;
-	int src_dir_nr = 0, src_dir_alloc = 0;
-	struct strbuf a_src_dir = STRBUF_INIT;
 	enum update_mode *modes, dst_mode = 0;
 	struct stat st, dest_st;
 	struct string_list src_for_dst = STRING_LIST_INIT_DUP;
@@ -323,8 +343,7 @@ dir_check:
 			/* last - first >= 1 */
 			modes[i] |= WORKING_DIRECTORY;
 
-			ALLOC_GROW(src_dir, src_dir_nr + 1, src_dir_alloc);
+			strvec_push(&src_dir, src);
-			src_dir[src_dir_nr++] = src;
 
 			n = argc + last - first;
 			REALLOC_ARRAY(modes, n);
@@ -538,25 +557,7 @@ remove_entry:
 		}
 	}
 
-	/*
+	remove_empty_src_dirs(src_dir.v, src_dir.nr);
-	 * cleanup the empty src_dirs
-	 */
-	for (i = 0; i < src_dir_nr; i++) {
-		int dummy;
-		strbuf_addstr(&a_src_dir, src_dir[i]);
-		/*
-		 * if entries under a_src_dir are all moved away,
-		 * recursively remove a_src_dir to cleanup
-		 */
-		if (index_range_of_same_dir(a_src_dir.buf, a_src_dir.len,
-					    &dummy, &dummy) < 1) {
-			remove_dir_recursively(&a_src_dir, 0);
-		}
-		strbuf_reset(&a_src_dir);
-	}
-
-	strbuf_release(&a_src_dir);
-	free(src_dir);
 
 	if (dirty_paths.nr)
 		advise_on_moving_dirty_path(&dirty_paths);
@@ -571,6 +572,7 @@ remove_entry:
 	ret = 0;
 
 out:
+	strvec_clear(&src_dir);
 	free(dst_w_slash);
 	string_list_clear(&src_for_dst, 0);
 	string_list_clear(&dirty_paths, 0);

t/unit-tests/t-strvec.c

@@ -4,6 +4,7 @@
 
 #define check_strvec(vec, ...) \
 	check_strvec_loc(TEST_LOCATION(), vec, __VA_ARGS__)
+LAST_ARG_MUST_BE_NULL
 static void check_strvec_loc(const char *loc, struct strvec *vec, ...)
 {
 	va_list ap;
@@ -22,11 +23,13 @@ static void check_strvec_loc(const char *loc, struct strvec *vec, ...)
 			strbuf_addf(&msg, "strvec index %"PRIuMAX, (uintmax_t) nr);
 			test_assert(loc, msg.buf, 0);
 			strbuf_release(&msg);
+			va_end(ap);
 			return;
 		}
 
 		nr++;
 	}
+	va_end(ap);
 
 	check_uint(vec->nr, ==, nr);
 	check_uint(vec->alloc, >=, nr);