Re: [PATCH 3/3] odb: drop gaps in object info flag values

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> +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.