On 24/10/23 11:55AM, Patrick Steinhardt wrote:
> diff --git a/reftable/basics.h b/reftable/basics.h
> index 7aa46d7c30d..86141602e74 100644
> --- a/reftable/basics.h
> +++ b/reftable/basics.h
> @@ -150,4 +150,11 @@ int common_prefix_size(struct reftable_buf *a, struct reftable_buf *b);
>
> int hash_size(uint32_t id);
>
> +/*
> + * Format IDs that identify the hash function used by a reftable. Note that
> + * these constants end up on disk and thus mustn't change.
> + */
> +#define REFTABLE_FORMAT_ID_SHA1 ((uint32_t) 0x73686131)
> +#define REFTABLE_FORMAT_ID_SHA256 ((uint32_t) 0x73323536)
This context is provided in the comments for `GIT_*_FORMAT_ID`, but now
that they being decoupled it might be nice to mention here that they
stand for "sha1" and "s256" respectively.
> +
> #endif
> diff --git a/reftable/reader.c b/reftable/reader.c
> index 90dc950b577..64eb6938efe 100644
> --- a/reftable/reader.c
> +++ b/reftable/reader.c
> @@ -109,16 +109,18 @@ static int parse_footer(struct reftable_reader *r, uint8_t *footer,
> if (r->version == 1) {
> r->hash_id = GIT_SHA1_FORMAT_ID;
> } else {
> - r->hash_id = get_be32(f);
> - switch (r->hash_id) {
> - case GIT_SHA1_FORMAT_ID:
> + switch (get_be32(f)) {
> + case REFTABLE_FORMAT_ID_SHA1:
> + r->hash_id = GIT_SHA1_FORMAT_ID;
> break;
> - case GIT_SHA256_FORMAT_ID:
> + case REFTABLE_FORMAT_ID_SHA256:
> + r->hash_id = GIT_SHA256_FORMAT_ID;
> break;
> default:
> err = REFTABLE_FORMAT_ERROR;
> goto done;
> }
Instead of directly mapping `hash_id` to `GIT_*_FORMAT_ID`, we use the
newly defined constants to map to the corresponding format. As an
internal identifier, the format ID can now be freely changed without
risk of affecting how on-disk reftable headers are parsed. Makes sense
to me and seems like a good change.
-Justin
