Re: [PATCH 3/3] odb: drop gaps in object info flag values
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 26, 2026, 16:58 UTC
- Message-ID
- <xmqqa4y0jop7.fsf@gitster.g>
- In-Reply-To
- <20260126-b4-pks-read-object-info-flags-v1-3-e682a003b17c@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
Show 27 quoted lines
> +enum object_info_flags {
> + /* Invoke lookup_replace_object() on the given hash. */
> + OBJECT_INFO_LOOKUP_REPLACE = (1 << 0),
> +
> + /* Do not reprepare object sources when the first lookup has failed. */
> + OBJECT_INFO_QUICK = (1 << 1),
> +
> + /*
> + * Do not attempt to fetch the object if missing (even if fetch_is_missing is
> + * nonzero).
> + */
> + OBJECT_INFO_SKIP_FETCH_OBJECT = (1 << 2),
> +
> + /* Die if object corruption (not just an object being missing) was detected. */
> + OBJECT_INFO_DIE_IF_CORRUPT = (1 << 3),
>
> -/* Die if object corruption (not just an object being missing) was detected. */
> -#define OBJECT_INFO_DIE_IF_CORRUPT 32
> + /*
> + * This is meant for bulk prefetching of missing blobs in a partial
> + * clone. Implies OBJECT_INFO_SKIP_FETCH_OBJECT and OBJECT_INFO_QUICK.
> + */
> + OBJECT_INFO_FOR_PREFETCH = (OBJECT_INFO_SKIP_FETCH_OBJECT | OBJECT_INFO_QUICK),
> +};
>
> /*
> * Read object info from the object database and populate the `object_info`I wonder if this series can be restructured a bit to demonstrate the benefit of moving to enum a bit more prominently. For example, even at the end of the three patches, odb_read_object_info_extended() still takes an "unsigned flags" parameter, but it is meant to take this new enum, isn't it? If we do the "#define to enum" conversion (without renumbering) first, then "unsigned to enum", would it, with appropriate compiler warning flags, already reveal the existing bugs that happened to be working OK as potential problems? And with that, fixes in 1/3 and 2/3 would demonstrate why #define to enum" is worth doing very well. And after all that, we can renumber the enums in a separate and final step.
Exactly the same comment applies to odb_has_object() that still takes "unsigned flags", even though HAS_OBJECT_* constants have already gone through the "#define to enum" conversion with an earier f8fc4cac (object-store: allow fetching objects via `has_object()`, 2025-04-29).
In any case, well spotted and nicely done. Thanks.