Re: [PATCH] Unify write_index_file functions

[Nicolas Pitre <nico@cam.org>]

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