Re: [PATCH 3/3] odb: drop gaps in object info flag values
- From
René Scharfe <l.s.r@web.de>
- Date
- Jan 26, 2026, 18:02 UTC
- Message-ID
- <add7c86f-9d5e-4136-8c3d-a04df523487b@web.de>
- In-Reply-To
- <xmqqa4y0jop7.fsf@gitster.g>
On 1/26/26 5:58 PM, Junio C Hamano wrote:
Show 41 quoted lines
> 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.With -Wenum-conversion you can get GCC to report implicit conversions between different enum types (like in the backfill case), but I don't see a way to warn about conversions from int (the fsck case).
https://stackoverflow.com/questions/4669454/how-to-make-gcc-warn-about-passing-wrong-enum-to-a-function suggests using -Wenum-compare and macros to sneak in a comparison, but that doesn't seem to catch more than -Wenum-conversion, which doesn't need any macros.
https://godbolt.org/z/Whvc7Mf1n
Perhaps sparse can do that?
René