Subject: Re: [PATCH v3 02/20] commit-graph: fix GRAPH_MIN_SIZE

From: Jakub Narebski <[email-masked]>
Date: Sat, 26 May 2018 20:46:09 +0200

Derrick Stolee <[email-masked]> writes:

> The GRAPH_MIN_SIZE macro should be the smallest size of a parsable
> commit-graph file. However, the minimum number of chunks was wrong.
> It is possible to write a commit-graph file with zero commits, and
> that violates this macro's value.
>
> Rewrite the macro, and use extra macros to better explain the magic
> constants.
>
> Signed-off-by: Derrick Stolee <[email-masked]>
> ---
>  commit-graph.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/commit-graph.c b/commit-graph.c
> index a8c337dd77..82295f0975 100644
> --- a/commit-graph.c
> +++ b/commit-graph.c
> @@ -33,10 +33,11 @@
>  
>  #define GRAPH_LAST_EDGE 0x80000000
>  
> +#define GRAPH_HEADER_SIZE 8

Nice.

>  #define GRAPH_FANOUT_SIZE (4 * 256)
>  #define GRAPH_CHUNKLOOKUP_WIDTH 12
> -#define GRAPH_MIN_SIZE (5 * GRAPH_CHUNKLOOKUP_WIDTH + GRAPH_FANOUT_SIZE + \
> -			GRAPH_OID_LEN + 8)
> +#define GRAPH_MIN_SIZE (GRAPH_HEADER_SIZE + 4 * GRAPH_CHUNKLOOKUP_WIDTH \
> +			+ GRAPH_FANOUT_SIZE + GRAPH_OID_LEN)

So in this case we have file header (4-byte signature, 1-byte version
number, 1-byte oid/hash version, 1-byte number of chunks, 1-byte
reserved: 4+1+1+1+1 = 8 bytes), chunk lookup: 3 required chunks plus
terminating label = 4 entries, constant-size fanout chunks, and
checksum.  Two remaining required chunks (OID Lookup and Commit Data)
can have length of 0.

One issue: in the future when Git moves to NewHash, it could encounter
then both commit-graph files using SHA-1 and using NewHash.  What about
GRPH_OID_LEN then: for one of those it would be incorrect.  Unless it is
about minimal length of checksum, that is we assume that NewHash would
be longer than SHA-1, but ten why name it GRAPH_OID_LEN?

This may be going too much in the future; there is no need to borrow
trouble now, where we have only SHA-1 supported as OID.  Still...

>  
>  char *get_commit_graph_filename(const char *obj_dir)
>  {

Best,
-- 
Jakub Narębski