From b259157f3c8705e0f775a3df2b33198499aede4f Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Wed, 23 May 2007 23:00:22 +0200 Subject: [PATCH 1/4] fast-import: Fix uninitialized variable Fix uninitialized last_object->no_free variable that is accessed in store_object. Signed-off-by: Simon Hausmann Signed-off-by: Shawn O. Pearce --- fast-import.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fast-import.c b/fast-import.c index 3a2d5ed8e6..03f5ca7998 100644 --- a/fast-import.c +++ b/fast-import.c @@ -1122,6 +1122,7 @@ static void store_tree(struct tree_entry *root) || le->pack_id != pack_id) { lo.data = NULL; lo.depth = 0; + lo.no_free = 0; } else { mktree(t, 0, &lo.len, &old_tree); lo.data = old_tree.buffer; From 20f546a86c5c673fc49050479a12edfb9f8672bf Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Wed, 23 May 2007 23:01:49 +0200 Subject: [PATCH 2/4] fast-import: Fix crash when referencing already existing objects Commit a5c1780a0355a71b9fb70f1f1977ce726ee5b8d8 sets the pack_id of existing objects to MAX_PACK_ID. When the same object is referenced later again it is found in the local object hash. With such a pack_id fast-import should not try to locate that object in the newly created pack(s). Signed-off-by: Simon Hausmann Signed-off-by: Shawn O. Pearce --- fast-import.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fast-import.c b/fast-import.c index 03f5ca7998..f308db7630 100644 --- a/fast-import.c +++ b/fast-import.c @@ -1013,7 +1013,7 @@ static void load_tree(struct tree_entry *root) return; myoe = find_object(sha1); - if (myoe) { + if (myoe && myoe->pack_id != MAX_PACK_ID) { if (myoe->type != OBJ_TREE) die("Not a tree: %s", sha1_to_hex(sha1)); t->delta_depth = 0; From 654aaa37ab5c70650bdd16d57b56c2d0f9aa43cf Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Thu, 24 May 2007 00:05:19 -0400 Subject: [PATCH 3/4] Refactor fast-import branch creation from existing commit To resolve a corner case uncovered by Simon Hausmann I need to reuse the logic for the SHA-1 expression version of the 'from ' command within the mark version of the 'from ' command. This change doesn't alter any functionality, but is merely breaking the common code out to a function that I can reuse. Signed-off-by: Shawn O. Pearce --- fast-import.c | 58 +++++++++++++++++++++++++++------------------------ 1 file changed, 31 insertions(+), 27 deletions(-) diff --git a/fast-import.c b/fast-import.c index f308db7630..f5107df74d 100644 --- a/fast-import.c +++ b/fast-import.c @@ -1657,6 +1657,33 @@ static void file_change_deleteall(struct branch *b) load_tree(&b->branch_tree); } +static void cmd_from_commit(struct branch *b, char *buf, unsigned long size) +{ + if (!buf || size < 46) + die("Not a valid commit: %s", sha1_to_hex(b->sha1)); + if (memcmp("tree ", buf, 5) + || get_sha1_hex(buf + 5, b->branch_tree.versions[1].sha1)) + die("The commit %s is corrupt", sha1_to_hex(b->sha1)); + hashcpy(b->branch_tree.versions[0].sha1, + b->branch_tree.versions[1].sha1); +} + +static void cmd_from_existing(struct branch *b) +{ + if (is_null_sha1(b->sha1)) { + hashclr(b->branch_tree.versions[0].sha1); + hashclr(b->branch_tree.versions[1].sha1); + } else { + unsigned long size; + char *buf; + + buf = read_object_with_reference(b->sha1, + commit_type, &size, b->sha1); + cmd_from_commit(b, buf, size); + free(buf); + } +} + static void cmd_from(struct branch *b) { const char *from; @@ -1688,34 +1715,11 @@ static void cmd_from(struct branch *b) die("Mark :%" PRIuMAX " not a commit", idnum); hashcpy(b->sha1, oe->sha1); buf = gfi_unpack_entry(oe, &size); - if (!buf || size < 46) - die("Not a valid commit: %s", from); - if (memcmp("tree ", buf, 5) - || get_sha1_hex(buf + 5, b->branch_tree.versions[1].sha1)) - die("The commit %s is corrupt", sha1_to_hex(b->sha1)); + cmd_from_commit(b, buf, size); free(buf); - hashcpy(b->branch_tree.versions[0].sha1, - b->branch_tree.versions[1].sha1); - } else if (!get_sha1(from, b->sha1)) { - if (is_null_sha1(b->sha1)) { - hashclr(b->branch_tree.versions[0].sha1); - hashclr(b->branch_tree.versions[1].sha1); - } else { - unsigned long size; - char *buf; - - buf = read_object_with_reference(b->sha1, - commit_type, &size, b->sha1); - if (!buf || size < 46) - die("Not a valid commit: %s", from); - if (memcmp("tree ", buf, 5) - || get_sha1_hex(buf + 5, b->branch_tree.versions[1].sha1)) - die("The commit %s is corrupt", sha1_to_hex(b->sha1)); - free(buf); - hashcpy(b->branch_tree.versions[0].sha1, - b->branch_tree.versions[1].sha1); - } - } else + } else if (!get_sha1(from, b->sha1)) + cmd_from_existing(b); + else die("Invalid ref name or SHA1 expression: %s", from); read_next_command(); From aac65ed1bc63619d32516079995f5cbe4bb46492 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Thu, 24 May 2007 00:32:31 -0400 Subject: [PATCH 4/4] Fix possible coredump with fast-import --import-marks When e8438420bb7d368bec3647b90c557b9931582267 allowed us to reload the marks table on subsequent runs of fast-import we really broke things, as we set pack_id to MAX_PACK_ID for any objects we imported into the marks table. Creating a branch from that mark should fail as we attempt to read the object through a non-existant packed_git pointer. Instead we have to use the normal Git object system to locate the older commit, as we ourselves do not have a reference to the packed_git it resides in. This bug only occurred because t9300 was not complete enough. When we added the --import-marks feature we didn't actually test its implementation enough to verify the function worked as intended. I have corrected that, and included the changes as part of this fix. Prior versions of fast-import fail the new test(s); this commit allows them to pass. Credit for this bug find goes to Simon Hausmann as he recently identified a similiar bug in the tree lazy-loading path. Signed-off-by: Shawn O. Pearce --- fast-import.c | 12 +++++++----- t/t9300-fast-import.sh | 29 +++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/fast-import.c b/fast-import.c index f5107df74d..17554f6849 100644 --- a/fast-import.c +++ b/fast-import.c @@ -1709,14 +1709,16 @@ static void cmd_from(struct branch *b) } else if (*from == ':') { uintmax_t idnum = strtoumax(from + 1, NULL, 10); struct object_entry *oe = find_mark(idnum); - unsigned long size; - char *buf; if (oe->type != OBJ_COMMIT) die("Mark :%" PRIuMAX " not a commit", idnum); hashcpy(b->sha1, oe->sha1); - buf = gfi_unpack_entry(oe, &size); - cmd_from_commit(b, buf, size); - free(buf); + if (oe->pack_id != MAX_PACK_ID) { + unsigned long size; + char *buf = gfi_unpack_entry(oe, &size); + cmd_from_commit(b, buf, size); + free(buf); + } else + cmd_from_existing(b); } else if (!get_sha1(from, b->sha1)) cmd_from_existing(b); else diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index 8e958da536..72e49f5d3b 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh @@ -119,6 +119,35 @@ test_expect_success \ input < $GIT_COMMITTER_DATE +data <expect <actual +test_expect_success \ + 'A: verify diff' \ + 'compare_diff_raw expect actual && + test `git-rev-parse --verify master:file2` \ + = `git-rev-parse --verify verify--import-marks:copy-of-file2`' + ### ### series B ###