Re: [PATCH] Unify write_index_file functions
From: Nicolas Pitre <hidden>
Date: 2016-06-15 22:43:13
On Fri, 1 Jun 2007, Geert Bosch wrote:
This patch creates a new pack-idx.c file containing a unified version of the write_index_file functions in builtin-pack-objects.c and index-pack.c. As the name "index" is overloaded in git, move in the direction of using "idx" and "pack idx" when refering to the pack index. There should be no change in functionality.
I intended to do exactly that (I even mentioned it in 81a216a5d6) but I'm glad you beat me to it. A few comments. Please use pack-write.c rather than a new file. This pack-write.c was created exactly to gather common pack writing tasks. Please also consider removing the pack index writing code from fast-import.c as well.
quoted hunk ↗ jump to hunk
@@ -24,9 +24,10 @@ git-pack-objects [{ -q | --progress | --all-progress }] [--max-pack-size=N] \n\ struct object_entry { unsigned char sha1[20]; - uint32_t crc32; /* crc of raw pack data for this object */ off_t offset; /* offset into the final pack file */ unsigned long size; /* uncompressed size */ + uint32_t crc32; /* crc of raw pack data for this object */
Don't do this. The crc32 field was carefully placed so the offset field is 64-bit aligned with no need for any padding. In fact, those 3 fields should probably be defined in a structure of their own rather than hoping that no one will fail to change the ordering in all places. Other than that it looks pretty good. Nicolas Nicolas