Review feedback

Author: lhchavez

src/midx.c

@@ -643,12 +643,7 @@ static int midx_write(
 	int error = 0;
 	size_t i;
 	struct git_pack_file *p;
-	struct git_midx_header hdr = {
-			.signature = htonl(MIDX_SIGNATURE),
-			.version = MIDX_VERSION,
-			.object_id_version = MIDX_OBJECT_ID_VERSION,
-			.base_midx_files = 0,
-	};
+	struct git_midx_header hdr = {0};
 	uint32_t oid_fanout_count;
 	uint32_t object_large_offsets_count;
 	uint32_t oid_fanout[256];
@@ -662,11 +657,16 @@ static int midx_write(
 	object_entry_array_t object_entries_array = GIT_ARRAY_INIT;
 	git_vector object_entries = GIT_VECTOR_INIT;
 	git_hash_ctx ctx;
-	struct midx_write_hash_context hash_cb_data = {
-		.write_cb = write_cb,
-		.cb_data = cb_data,
-		.ctx = &ctx,
-	};
+	struct midx_write_hash_context hash_cb_data = {0};
+
+	hdr.signature = htonl(MIDX_SIGNATURE);
+	hdr.version = MIDX_VERSION;
+	hdr.object_id_version = MIDX_OBJECT_ID_VERSION;
+	hdr.base_midx_files = 0;
+
+	hash_cb_data.write_cb = write_cb;
+	hash_cb_data.cb_data = cb_data;
+	hash_cb_data.ctx = &ctx;
 
 	error = git_hash_ctx_init(&ctx);
 	if (error < 0)
@@ -677,12 +677,12 @@ static int midx_write(
 	git_vector_sort(&w->packs);
 	git_vector_foreach (&w->packs, i, p) {
 		git_buf relative_index = GIT_BUF_INIT;
-		struct object_entry_cb_state state = {
-				.pack_index = (uint32_t)i,
-				.object_entries_array = &object_entries_array,
-		};
+		struct object_entry_cb_state state = {0};
 		size_t path_len;
 
+		state.pack_index = (uint32_t)i;
+		state.object_entries_array = &object_entries_array;
+
 		error = git_buf_sets(&relative_index, p->pack_name);
 		if (error < 0)
 			goto cleanup;
@@ -694,6 +694,8 @@ static int midx_write(
 		path_len = git_buf_len(&relative_index);
 		if (path_len <= strlen(".pack") || git__suffixcmp(git_buf_cstr(&relative_index), ".pack") != 0) {
 			git_buf_dispose(&relative_index);
+			git_error_set(GIT_ERROR_INVALID, "invalid packfile name: '%s'", p->pack_name);
+			error = -1;
 			goto cleanup;
 		}
 		path_len -= strlen(".pack");

src/pack.c

@@ -1396,6 +1396,7 @@ int git_pack_foreach_entry_offset(
 
 	index += 4 * 256;
 
+	/* all offsets should have been validated by pack_index_check_locked */
 	if (p->index_version > 1) {
 		const unsigned char *offsets = index + 24 * p->num_objects;
 		const unsigned char *large_offset_ptr;
@@ -1406,7 +1407,7 @@ int git_pack_foreach_entry_offset(
 			if (current_offset & 0x80000000) {
 				large_offset_ptr = large_offsets + (current_offset & 0x7fffffff) * 8;
 				if (large_offset_ptr >= large_offsets_end) {
-					error = -1;
+					error = packfile_error("invalid large offset");
 					goto cleanup;
 				}
 				current_offset = (((off64_t)ntohl(*((uint32_t *)(large_offset_ptr + 0)))) << 32) |

src/pack.h

@@ -23,7 +23,7 @@
 /**
  * Function type for callbacks from git_pack_foreach_entry_offset.
  */
-typedef int GIT_CALLBACK(git_pack_foreach_entry_offset_cb)(
+typedef int git_pack_foreach_entry_offset_cb(
 		const git_oid *id,
 		off64_t offset,
 		void *payload);
@@ -185,8 +185,11 @@ int git_pack_foreach_entry(
 		git_odb_foreach_cb cb,
 		void *data);
 /**
- * Similar to git_pack_foreach_entry, but it also provides the offset of the
- * object within the packfile. It also does not sort the objects in any order.
+ * Similar to git_pack_foreach_entry, but:
+ * - It also provides the offset of the object within the
+ *   packfile.
+ * - It does not sort the objects in any order.
+ * - It retains the lock while invoking the callback.
  */
 int git_pack_foreach_entry_offset(
 		struct git_pack_file *p,