From: Justin Tobler <jltobler@gmail.com>
Date: Mon, 09 Feb 2026 20:04:29 GMT
Subject: Re: [PATCH 2/3] builtin/fsck: fix flags passed to `odb_has_object()`
Message-ID: <aYo8QoT2y8s_0itJ@denethor>
In-Reply-To: <20260126-b4-pks-read-object-info-flags-v1-2-e682a003b17c@pks.im>

On 26/01/26 01:17PM, Patrick Steinhardt wrote:
> In `mark_object()` we invoke `has_object()` with a value of 1. This is
> somewhat fishy given that the function expects a bitset of flags, so any
> behaviour that this results in is purely coincidental and may break at
> any point in time.
> 
> The call to `has_object()` was originally introduced in 9eb86f41de
> (fsck: do not lazy fetch known non-promisor object, 2020-08-05). The
> intent here was to skip lazy fetches of promisor objects: we have
> already verified that the object is not a promisor object, so if the
> object is missing it indicates a corrupt repository.
> 
> The hardcoded value that we pass maps to `HAS_OBJECT_RECHECK_PACKED`,
> which is probably the intended behaviour: `odb_has_object()` will not
> fetch promisor objects unless `HAS_OBJECT_FETCH_PROMISOR` is passed, but
> we may want to verify that no concurrent process has written the object
> that we're trying to read.

As you mentioned, promisor objects are not fetched unless
`HAS_OBJECT_FETCH_PROMISOR` is passed and in this case a flag value of 1
maps only to the `HAS_OBJECT_RECHECK_PACKED` flag. This certainly seems
like the intended option.

> Convert the code to use the named flag instead of the the hardcoded
> value.

Makes sense, this patch looks good to me.

-Justin