git-mv: fix moving more than one source to a single destination

The code used as if return value from basename(3) were stable, but
often the function is implemented to return a pointer to a static
storage internal to it.

Because basename(3) is also allowed to modify its input parameter in
place, casting constness away from the strings we obtained from the
caller and giving them to basename is a no-no.

Reported, and initial fix and test supplied by David Rydh.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin-mv.c b/builtin-mv.c
index 82471869a0b677202fb5585e2fca880d16478af8..c07f53b34380200eaba223d2e1f8f977c1f9fd18 100644 (file)
--- a/builtin-mv.c
+++ b/builtin-mv.c
@@ -24,10 +24,13 @@ static const char **copy_pathspec(const char *prefix, const char **pathspec,
        result[count] = NULL;
        for (i = 0; i < count; i++) {
                int length = strlen(result[i]);
-               if (length > 0 && is_dir_sep(result[i][length - 1]))
-                       result[i] = xmemdupz(result[i], length - 1);
-               if (base_name)
-                       result[i] = basename((char *)result[i]);
+               int to_copy = length;
+               while (to_copy > 0 && is_dir_sep(result[i][to_copy - 1]))
+                       to_copy--;
+               if (to_copy != length || base_name) {
+                       char *it = xmemdupz(result[i], to_copy);
+                       result[i] = base_name ? strdup(basename(it)) : it;
+               }
        }
        return get_pathspec(prefix, result);
 }

diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index 10b8f8c44befdb4eb00b3959f8b29cbebb7a22e1..65a35d94a001b555ce9e4d6c528d588339a5300b 100755 (executable)
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -189,6 +189,18 @@ test_expect_success 'absolute pathname outside should fail' '(
 
 )'
 
+test_expect_success 'git mv to move multiple sources into a directory' '
+       rm -fr .git && git init &&
+       mkdir dir other &&
+       >dir/a.txt &&
+       >dir/b.txt &&
+       git add dir/?.txt &&
+       git mv dir/a.txt dir/b.txt other &&
+       git ls-files >actual &&
+       { echo other/a.txt; echo other/b.txt; } >expect &&
+       test_cmp expect actual
+'
+
 test_expect_success 'git mv should not change sha1 of moved cache entry' '
 
        rm -fr .git &&