Re: [PATCH 3/3] odb: drop gaps in object info flag values
On 26/01/26 01:17PM, Patrick Steinhardt wrote:
Show 18 quoted lines
> 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.
Show 51 quoted lines
> 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