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

[Justin Tobler <jltobler@gmail.com>]

On 26/01/26 01:17PM, Patrick Steinhardt wrote:
> The object info flag values have a two gaps in their definitions, where
> some bits are skipped over. These gaps don't really hurt, but it makes
> one wonder whether anything is going on and whether a subset of flags
> might be defined somewhere else.
> 
> That's not the case though. Instead, this is a case of flags that have
> been dropped in the past:
> 
>   - The value 4 was used by `OBJECT_INFO_SKIP_CACHED`, removed in
>     9c8a294a1a (sha1-file: remove OBJECT_INFO_SKIP_CACHED, 2020-01-02).
> 
>   - The value 8 was used by `OBJECT_INFO_ALLOW_UNKNOWN_TYPE`, removed in
>     ae24b032a0 (object-file: drop OBJECT_INFO_ALLOW_UNKNOWN_TYPE flag,
>     2025-05-16).
> 
> Close those gaps to avoid any more confusion. While at it, convert the
> flags to be declared as an enum and use bit shifts to follow modern best
> practices.

Makes sense.

> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  odb.h | 38 ++++++++++++++++++++++----------------
>  1 file changed, 22 insertions(+), 16 deletions(-)
> 
> diff --git a/odb.h b/odb.h
> index bab07755f4..1e4326b7f4 100644
> --- a/odb.h
> +++ b/odb.h
> @@ -352,23 +352,29 @@ struct object_info {
>   */
>  #define OBJECT_INFO_INIT { 0 }
>  
> -/* Invoke lookup_replace_object() on the given hash */
> -#define OBJECT_INFO_LOOKUP_REPLACE 1
> -/* Do not retry packed storage after checking packed and loose storage */
> -#define OBJECT_INFO_QUICK 8
> -/*
> - * Do not attempt to fetch the object if missing (even if fetch_is_missing is
> - * nonzero).
> - */
> -#define OBJECT_INFO_SKIP_FETCH_OBJECT 16
> -/*
> - * This is meant for bulk prefetching of missing blobs in a partial
> - * clone. Implies OBJECT_INFO_SKIP_FETCH_OBJECT and OBJECT_INFO_QUICK
> - */
> -#define OBJECT_INFO_FOR_PREFETCH (OBJECT_INFO_SKIP_FETCH_OBJECT | OBJECT_INFO_QUICK)
> +/* Flags that can be passed to `odb_read_object_info_extended()`. */
> +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),

The changes here all look obviously correct to me. Looks good.

-Justin