Derrick Stolee <dsto...@microsoft.com> 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 <dsto...@microsoft.com>
> ---
> 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
