Re: [PATCH 2/2] fast-import: teach ls command to accept empty path
From: David Barr <hidden>
Date: 2016-06-15 22:53:15
Subsystem:
the rest · Maintainer:
Linus Torvalds
On Fri, Mar 9, 2012 at 7:33 AM, Jonathan Nieder [off-list ref] wrote:
From: David Barr <redacted> 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 <redacted> Signed-off-by: David Barr <redacted> Signed-off-by: Dmitry Ivankov <redacted> Signed-off-by: Jonathan Nieder <redacted> --- + /* + * 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