On 2/7/2021 4:13 PM, SZEDER Gábor wrote: >> +#define CHUNK_LOOKUP_WIDTH 12 > > As this macro is defined in 'chunk-format.c' it's not part of the > chunkfile API. However, at the end of this patch series > 'commit-graph.c' still contains: > > #define GRAPH_CHUNKLOOKUP_WIDTH 12 > > and uses it in a couple of safety checks (that didn't became part of > the common chunkfile module; why?), Chunk-based files don't have a minimum size unless we know the header size and a minimum number of required chunks. I suppose that we could add this in the future to further simplify consumers of the API. > while 'midx.c' contains: > > #define MIDX_CHUNKLOOKUP_WIDTH (sizeof(uint32_t) + sizeof(uint64_t)) > > though it's not used anymore. > > I think we should have only one such constant as part of the chunkfile > API; and preferably use the definition from 'midx.c' as it is more > informative than yet another magic number. > > Furthermore, being called 'CHUNK_LOOKUP_WIDTH', I had to look up the > places where this constant is used to make sure that it indeed means > what I suspect it means. Perhaps CHUNK_TOC_ENTRY_SIZE would be a more > descriptive name for this constant. More descriptive, for sure. > On a somewhat related note: 'commit-graph.c' and 'midx.c' still > contains the constants MAX_NUM_CHUNKS and MIDX_MAX_CHUNKS, > respecticely, but neither of them is used anymore. Thanks. The following patch can be added on top of this series to clean up these dangling macros. Thanks, -Stolee --- >8 --- >From 839b880ccee65eac63e8b77b12fab6531acc55b0 Mon Sep 17 00:00:00 2001 From: Derrick Stolee <dstolee@xxxxxxxxxxxxx> Date: Mon, 8 Feb 2021 08:38:47 -0500 Subject: [PATCH] chunk-format: remove outdated macro constants MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The following macros were needed by midx.c and commit-graph.c to handle their independent implementations of the chunk-based file format, but now the chunk-format API makes them obsolete: * MAX_NUM_CHUNKS * MIDX_MAX_CHUNKS * MIX_CHUNKLOOKUP_WIDTH Reported-by: SZEDER Gábor <szeder.dev@xxxxxxxxx> Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx> --- commit-graph.c | 1 - midx.c | 2 -- 2 files changed, 3 deletions(-) diff --git a/commit-graph.c b/commit-graph.c index 32cf5091d2f..3b5a8767269 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -45,7 +45,6 @@ void git_test_write_commit_graph_or_die(void) #define GRAPH_CHUNKID_BLOOMINDEXES 0x42494458 /* "BIDX" */ #define GRAPH_CHUNKID_BLOOMDATA 0x42444154 /* "BDAT" */ #define GRAPH_CHUNKID_BASE 0x42415345 /* "BASE" */ -#define MAX_NUM_CHUNKS 9 #define GRAPH_DATA_WIDTH (the_hash_algo->rawsz + 16) diff --git a/midx.c b/midx.c index 95648a1f368..5c7f2ed2333 100644 --- a/midx.c +++ b/midx.c @@ -22,14 +22,12 @@ #define MIDX_HEADER_SIZE 12 #define MIDX_MIN_SIZE (MIDX_HEADER_SIZE + the_hash_algo->rawsz) -#define MIDX_MAX_CHUNKS 5 #define MIDX_CHUNK_ALIGNMENT 4 #define MIDX_CHUNKID_PACKNAMES 0x504e414d /* "PNAM" */ #define MIDX_CHUNKID_OIDFANOUT 0x4f494446 /* "OIDF" */ #define MIDX_CHUNKID_OIDLOOKUP 0x4f49444c /* "OIDL" */ #define MIDX_CHUNKID_OBJECTOFFSETS 0x4f4f4646 /* "OOFF" */ #define MIDX_CHUNKID_LARGEOFFSETS 0x4c4f4646 /* "LOFF" */ -#define MIDX_CHUNKLOOKUP_WIDTH (sizeof(uint32_t) + sizeof(uint64_t)) #define MIDX_CHUNK_FANOUT_SIZE (sizeof(uint32_t) * 256) #define MIDX_CHUNK_OFFSET_WIDTH (2 * sizeof(uint32_t)) #define MIDX_CHUNK_LARGE_OFFSET_WIDTH (sizeof(uint64_t)) -- 2.30.0.vfs.0.0.exp
