From 734c91f9e292cf6ed1401c178bc9fbb902cc82dd Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Mon, 5 Mar 2007 12:31:09 -0500 Subject: fast-import: Avoid infinite loop after reset Johannes Sixt noticed that a 'reset' command applied to a branch that is already active in the branch LRU cache can cause fast-import to relink the same branch into the LRU cache twice. This will cause the LRU cache to contain a cycle, making unload_one_branch run in an infinite loop as it tries to select the oldest branch for eviction. I have trivially fixed the problem by adding an active bit to each branch object; this bit indicates if the branch is already in the LRU and allows us to avoid trying to add it a second time. Converting the pack_id field into a bitfield makes this change take up no additional memory. Signed-off-by: Shawn O. Pearce --- fast-import.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) (limited to 'fast-import.c') diff --git a/fast-import.c b/fast-import.c index 1ae125a04..490f64003 100644 --- a/fast-import.c +++ b/fast-import.c @@ -220,7 +220,8 @@ struct branch const char *name; struct tree_entry branch_tree; uintmax_t last_commit; - unsigned int pack_id; + unsigned active : 1; + unsigned pack_id : PACK_ID_BITS; unsigned char sha1[20]; }; @@ -528,6 +529,7 @@ static struct branch *new_branch(const char *name) b->table_next_branch = branch_table[hc]; b->branch_tree.versions[0].mode = S_IFDIR; b->branch_tree.versions[1].mode = S_IFDIR; + b->active = 0; b->pack_id = MAX_PACK_ID; branch_table[hc] = b; branch_count++; @@ -1547,6 +1549,7 @@ static void unload_one_branch(void) e = active_branches; active_branches = e->active_next_branch; } + e->active = 0; e->active_next_branch = NULL; if (e->branch_tree.tree) { release_tree_content_recursive(e->branch_tree.tree); @@ -1559,10 +1562,13 @@ static void unload_one_branch(void) static void load_branch(struct branch *b) { load_tree(&b->branch_tree); - b->active_next_branch = active_branches; - active_branches = b; - cur_active_branches++; - branch_load_count++; + if (!b->active) { + b->active = 1; + b->active_next_branch = active_branches; + active_branches = b; + cur_active_branches++; + branch_load_count++; + } } static void file_change_m(struct branch *b) -- cgit v1.2.1 From 2f6dc35d2ad0bd2a8648902a692f087f47d1ee86 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Mon, 5 Mar 2007 12:43:14 -0500 Subject: fast-import: Fail if a non-existant commit is used for merge Johannes Sixt noticed during one of his own imports that fast-import did not fail if a non-existant commit is referenced by SHA-1 value as an argument to the 'merge' command. This allowed the user to unknowingly create commits that would fail in fsck, as the commit contents would not be completely reachable. A side effect of this bug was that a frontend process could mark any SHA-1 object (blob, tree, tag) as a parent of a merge commit. This should also fail in fsck, as the commit is not a valid commit. We now use the same rule as the 'from' command. If a commit is referenced in the 'merge' command by hex formatted SHA-1 then the SHA-1 must be a commit or a tag that can be peeled back to a commit, the commit must already exist, and must be readable by the core Git infrastructure code. This requirement means that the commit must have existed prior to fast-import starting, or the commit must have been flushed out by a prior 'checkpoint' command. Signed-off-by: Shawn O. Pearce --- fast-import.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'fast-import.c') diff --git a/fast-import.c b/fast-import.c index 490f64003..d9492b988 100644 --- a/fast-import.c +++ b/fast-import.c @@ -1752,7 +1752,14 @@ static struct hash_list *cmd_merge(unsigned int *count) if (oe->type != OBJ_COMMIT) die("Mark :%" PRIuMAX " not a commit", idnum); hashcpy(n->sha1, oe->sha1); - } else if (get_sha1(from, n->sha1)) + } else if (!get_sha1(from, n->sha1)) { + unsigned long size; + char *buf = read_object_with_reference(n->sha1, + type_names[OBJ_COMMIT], &size, n->sha1); + if (!buf || size < 46) + die("Not a valid commit: %s", from); + free(buf); + } else die("Invalid ref name or SHA1 expression: %s", from); n->next = NULL; -- cgit v1.2.1