On Fri, Mar 9, 2012 at 7:33 AM, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote:
> From: David Barr <davidbarr@xxxxxxxxxx>
>
> There is a pathological Subversion operation that svn-fe handles
> incorrectly due to an unexpected response from fast-import:
>
> svn cp $SVN_ROOT $SVN_ROOT/subdirectory
>
> When the following command is sent to fast-import:
>
> 'ls' SP ':1' SP LF
>
> The expected output is:
>
> '040000' SP 'tree' SP <dataref> HT LF
>
> The actual output is:
>
> 'missing' SP LF
>
> This is because tree_content_get() is called but expects a non-empty
> path. Instead, copy the root entry.
>
> [jn: using a deep copy; w/ more tests]
> [jn: with a fix from Dmitry to fully initialize root->versions[0]
> and versions[1] now that root can be passed to store_tree]
>
> Reported-by: Andrew Sayers <andrew-git@xxxxxxxxxxxxxxx>
> Signed-off-by: David Barr <davidbarr@xxxxxxxxxx>
> Signed-off-by: Dmitry Ivankov <divanorama@xxxxxxxxx>
> Signed-off-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
> ---
> + /*
> + * store_tree scribbles over version[0] in leaf.tree's
> + * entries, so we need a deep copy.
> + */
> + if (root->tree && is_null_sha1(root->versions[1].sha1))
> + leaf.tree = dup_tree_content(root->tree);
Is it ok to call store_tree(root)? If so, could we not introduce a
pointer rather than a deep copy?
diff --git a/fast-import.c b/fast-import.c
index 94d7037..eab24f3 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2993,7 +2993,8 @@ static void parse_ls(struct branch *b)
{
const char *p;
struct tree_entry *root = NULL;
- struct tree_entry leaf = {NULL};
+ struct tree_entry tmp_tree = {NULL};
+ struct tree_entry *leaf = &tmp_tree;
/* ls SP (<treeish> SP)? <path> */
p = command_buf.buf + strlen("ls ");
@@ -3022,27 +3023,18 @@ static void parse_ls(struct branch *b)
die("Garbage after path in: %s", command_buf.buf);
p = uq.buf;
}
- if (*p) {
- tree_content_get(root, p, &leaf);
- } else {
- memcpy(&leaf, root, sizeof(leaf));
- /*
- * store_tree scribbles over version[0] in leaf.tree's
- * entries, so we need a deep copy.
- */
- if (root->tree && is_null_sha1(root->versions[1].sha1))
- leaf.tree = dup_tree_content(root->tree);
- else
- leaf.tree = NULL;
- }
+ if (*p)
+ tree_content_get(root, p, leaf);
+ else
+ leaf = root;
/*
* A directory in preparation would have a sha1 of zero
* until it is saved. Save, for simplicity.
*/
- if (S_ISDIR(leaf.versions[1].mode))
- store_tree(&leaf);
+ if (S_ISDIR(leaf->versions[1].mode))
+ store_tree(leaf);
- print_ls(leaf.versions[1].mode, leaf.versions[1].sha1, p);
+ print_ls(leaf->versions[1].mode, leaf->versions[1].sha1, p);
if (!b || root != &b->branch_tree)
release_tree_entry(root);
}
--
David Barr
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html