From: Nicolas Pitre Date: Fri, 01 Jun 2007 20:54:38 GMT Subject: Re: [PATCH] Unify write_index_file functions Message-ID: In-Reply-To: <20070601194856.66DFB4D7206@potomac.gnat.com> 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. > @@ -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