pack: simplify delta chain code The switch makes the loop somewhat unwieldy. Let's assume it's fine and perform the check when we're accessing the data. This makes our code look a lot more like git's.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152
diff --git a/src/pack.c b/src/pack.c
index 664e8ef..e1fd276 100644
--- a/src/pack.c
+++ b/src/pack.c
@@ -549,21 +549,38 @@ int git_packfile_unpack(
/* the first one is the base, so we expand that one */
elem = git_array_pop(chain);
+ base_type = elem->type;
if (elem->cached) {
cached = elem->cached_entry;
memcpy(obj, &cached->raw, sizeof(git_rawobj));
- base_type = obj->type;
- } else {
- curpos = elem->offset;
- error = packfile_unpack_compressed(obj, p, &w_curs, &curpos, elem->size, elem->type);
- git_mwindow_close(&w_curs);
- base_type = elem->type;
- free_base = 1;
}
if (error < 0)
goto cleanup;
+ switch (base_type) {
+ case GIT_OBJ_COMMIT:
+ case GIT_OBJ_TREE:
+ case GIT_OBJ_BLOB:
+ case GIT_OBJ_TAG:
+ if (!elem->cached) {
+ curpos = elem->offset;
+ error = packfile_unpack_compressed(obj, p, &w_curs, &curpos, elem->size, elem->type);
+ git_mwindow_close(&w_curs);
+ free_base = 1;
+ }
+ if (error < 0)
+ goto cleanup;
+ break;
+ case GIT_OBJ_OFS_DELTA:
+ case GIT_OBJ_REF_DELTA:
+ error = packfile_error("dependency chain ends in a delta");
+ goto cleanup;
+ default:
+ error = packfile_error("invalid packfile type in header");
+ goto cleanup;
+ }
+
/*
* Finding the object we want as the base element is
* problematic, as we need to make sure we don't accidentally
@@ -1229,7 +1246,7 @@ static int pack_dependency_chain(git_dependency_chain *chain_out, struct git_pac
git_dependency_chain chain = GIT_ARRAY_INIT;
git_mwindow *w_curs = NULL;
git_off_t curpos = obj_offset, base_offset;
- int error = 0, found_base = 0;
+ int error = 0;
size_t size;
git_otype type;
@@ -1237,7 +1254,7 @@ static int pack_dependency_chain(git_dependency_chain *chain_out, struct git_pac
return -1;
git_array_init_to_size(chain, 64);
- while (!found_base && error == 0) {
+ while (true) {
struct pack_chain_elem *elem;
git_pack_cache_entry *cached = NULL;
@@ -1248,6 +1265,16 @@ static int pack_dependency_chain(git_dependency_chain *chain_out, struct git_pac
goto on_error;
}
+ elem->base_key = obj_offset;
+
+ /* if we have a base cached, we can stop here instead */
+ if ((cached = cache_get(&p->bases, obj_offset)) != NULL) {
+ elem->cached_entry = cached;
+ elem->cached = 1;
+ elem->type = cached->raw.type;
+ break;
+ }
+
error = git_packfile_unpack_header(&size, &type, &p->mwf, &w_curs, &curpos);
git_mwindow_close(&w_curs);
@@ -1260,51 +1287,26 @@ static int pack_dependency_chain(git_dependency_chain *chain_out, struct git_pac
elem->type = type;
elem->base_key = obj_offset;
- switch (type) {
- case GIT_OBJ_OFS_DELTA:
- case GIT_OBJ_REF_DELTA:
- /* if we have a base cached, we can stop here instead */
- if ((cached = cache_get(&p->bases, obj_offset)) != NULL) {
- elem->cached_entry = cached;
- elem->cached = 1;
- found_base = 1;
- break;
- }
-
- base_offset = get_delta_base(p, &w_curs, &curpos, type, obj_offset);
- git_mwindow_close(&w_curs);
-
- if (base_offset == 0)
- return packfile_error("delta offset is zero");
- if (base_offset < 0) { /* must actually be an error code */
- error = (int)base_offset;
- goto on_error;
- }
-
- /* we need to pass the pos *after* the delta-base bit */
- elem->offset = curpos;
-
- /* go through the loop again, but with the new object */
- obj_offset = base_offset;
+ if (type != GIT_OBJ_OFS_DELTA && type != GIT_OBJ_REF_DELTA)
break;
- /* one of these means we've found the final object in the chain */
- case GIT_OBJ_COMMIT:
- case GIT_OBJ_TREE:
- case GIT_OBJ_BLOB:
- case GIT_OBJ_TAG:
- found_base = 1;
- break;
+ base_offset = get_delta_base(p, &w_curs, &curpos, type, obj_offset);
+ git_mwindow_close(&w_curs);
- default:
- error = packfile_error("invalid packfile type in header");
- break;
+ if (base_offset == 0) {
+ error = packfile_error("delta offset is zero");
+ goto on_error;
}
- }
+ if (base_offset < 0) { /* must actually be an error code */
+ error = (int)base_offset;
+ goto on_error;
+ }
+
+ /* we need to pass the pos *after* the delta-base bit */
+ elem->offset = curpos;
- if (!found_base) {
- git_array_clear(chain);
- return packfile_error("after dependency chain loop; cannot happen");
+ /* go through the loop again, but with the new object */
+ obj_offset = base_offset;
}
*chain_out = chain;