From 048357d49d59cbb8c81ff891803d5eace813baef Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 12 Feb 2026 07:59:37 +0100 Subject: [PATCH 1/5] builtin/backfill: fix flags passed to `odb_has_object()` The function `fill_missing_blobs()` receives an array of object IDs and verifies for each of them whether the corresponding object exists. If it doesn't exist, we add it to a set of objects and then batch-fetch all of the objects at once. The check for whether or not we already have the object is broken though: we pass `OBJECT_INFO_FOR_PREFETCH`, but `odb_has_object()` expects us to pass `HAS_OBJECT_*` flags. The flag expands to: - `OBJECT_INFO_QUICK`, which asks the object database to not reprepare in case the object wasn't found. This makes sense, as we'd otherwise reprepare the object database as many times as we have missing objects. - `OBJECT_INFO_SKIP_FETCH_OBJECT`, which asks the object database to not fetch the object in case it's missing. Again, this makes sense, as we want to batch-fetch the objects. This shows that we indeed want the equivalent of this flag, but of course represented as `HAS_OBJECT_*` flags. Luckily, the code is already working correctly. The `OBJECT_INFO` flag expands to `(1 << 3) | (1 << 4)`, none of which are valid `HAS_OBJECT` flags. And if no flags are passed, `odb_has_object()` ends up calling `odb_read_object_info_extended()` with exactly the above two flags that we wanted to set in the first place. Of course, this is pure luck, and this can break any moment. So let's fix this and correct the code to not pass any flags at all. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/backfill.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/builtin/backfill.c b/builtin/backfill.c index e80fc1b694..d8cb3b0eba 100644 --- a/builtin/backfill.c +++ b/builtin/backfill.c @@ -67,8 +67,7 @@ static int fill_missing_blobs(const char *path UNUSED, return 0; for (size_t i = 0; i < list->nr; i++) { - if (!odb_has_object(ctx->repo->objects, &list->oid[i], - OBJECT_INFO_FOR_PREFETCH)) + if (!odb_has_object(ctx->repo->objects, &list->oid[i], 0)) oid_array_append(&ctx->current_batch, &list->oid[i]); } From 6cf965ccaf04cfaa0d1bc72336c380970549c6ee Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 12 Feb 2026 07:59:38 +0100 Subject: [PATCH 2/5] builtin/fsck: fix flags passed to `odb_has_object()` 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. Convert the code to use the named flag instead of the the hardcoded value. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fsck.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/builtin/fsck.c b/builtin/fsck.c index 0512f78a87..1d059dd6c2 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -162,7 +162,8 @@ static int mark_object(struct object *obj, enum object_type type, return 0; if (!(obj->flags & HAS_OBJ)) { - if (parent && !odb_has_object(the_repository->objects, &obj->oid, 1)) { + if (parent && !odb_has_object(the_repository->objects, &obj->oid, + HAS_OBJECT_RECHECK_PACKED)) { printf_ln(_("broken link from %7s %s\n" " to %7s %s"), printable_type(&parent->oid, parent->type), From ae77afc3478c87766c4db9082fa82195d6ce9560 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 12 Feb 2026 07:59:39 +0100 Subject: [PATCH 3/5] odb: drop gaps in object info flag values 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. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- odb.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/odb.h b/odb.h index bab07755f4..8e1fca7755 100644 --- a/odb.h +++ b/odb.h @@ -353,14 +353,14 @@ struct object_info { #define OBJECT_INFO_INIT { 0 } /* Invoke lookup_replace_object() on the given hash */ -#define OBJECT_INFO_LOOKUP_REPLACE 1 +#define OBJECT_INFO_LOOKUP_REPLACE (1 << 0) /* Do not retry packed storage after checking packed and loose storage */ -#define OBJECT_INFO_QUICK 8 +#define OBJECT_INFO_QUICK (1 << 1) /* * Do not attempt to fetch the object if missing (even if fetch_is_missing is * nonzero). */ -#define OBJECT_INFO_SKIP_FETCH_OBJECT 16 +#define OBJECT_INFO_SKIP_FETCH_OBJECT (1 << 2) /* * This is meant for bulk prefetching of missing blobs in a partial * clone. Implies OBJECT_INFO_SKIP_FETCH_OBJECT and OBJECT_INFO_QUICK @@ -368,7 +368,7 @@ struct object_info { #define OBJECT_INFO_FOR_PREFETCH (OBJECT_INFO_SKIP_FETCH_OBJECT | OBJECT_INFO_QUICK) /* Die if object corruption (not just an object being missing) was detected. */ -#define OBJECT_INFO_DIE_IF_CORRUPT 32 +#define OBJECT_INFO_DIE_IF_CORRUPT (1 << 3) /* * Read object info from the object database and populate the `object_info` From f6516a5241684355f3fb9f7b70e287e98b48d0ef Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 12 Feb 2026 07:59:40 +0100 Subject: [PATCH 4/5] odb: convert object info flags into an enum Convert the object info flags into an enum and adapt all functions that receive these flags as parameters to use the enum instead of an integer. This serves two purposes: - The function signatures become more self-documenting, as callers don't have to wonder which flags they expect. - The compiler can warn when a wrong flag type is passed. Note that the second benefit is somewhat limited. For example, when or-ing multiple enum flags together the result will be an integer, and the compiler will not warn about such use cases. But where it does help is when a single flag of the wrong type is passed, as the compiler would generate a warning in that case. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- object-file.c | 3 ++- object-file.h | 3 ++- odb.c | 2 +- odb.h | 40 +++++++++++++++++++++++----------------- packfile.c | 2 +- packfile.h | 2 +- 6 files changed, 30 insertions(+), 22 deletions(-) diff --git a/object-file.c b/object-file.c index e7e4c3348f..0ab6c4d4f3 100644 --- a/object-file.c +++ b/object-file.c @@ -414,7 +414,8 @@ static int parse_loose_header(const char *hdr, struct object_info *oi) int odb_source_loose_read_object_info(struct odb_source *source, const struct object_id *oid, - struct object_info *oi, int flags) + struct object_info *oi, + enum object_info_flags flags) { int ret; int fd; diff --git a/object-file.h b/object-file.h index 1229d5f675..cdb54b5218 100644 --- a/object-file.h +++ b/object-file.h @@ -47,7 +47,8 @@ void odb_source_loose_reprepare(struct odb_source *source); int odb_source_loose_read_object_info(struct odb_source *source, const struct object_id *oid, - struct object_info *oi, int flags); + struct object_info *oi, + enum object_info_flags flags); int odb_source_loose_read_object_stream(struct odb_read_stream **out, struct odb_source *source, diff --git a/odb.c b/odb.c index ac70b6a099..d437aa8b06 100644 --- a/odb.c +++ b/odb.c @@ -842,7 +842,7 @@ static int oid_object_info_convert(struct repository *r, int odb_read_object_info_extended(struct object_database *odb, const struct object_id *oid, struct object_info *oi, - unsigned flags) + enum object_info_flags flags) { int ret; diff --git a/odb.h b/odb.h index 8e1fca7755..e94cdc3665 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 << 0) -/* Do not retry packed storage after checking packed and loose storage */ -#define OBJECT_INFO_QUICK (1 << 1) -/* - * Do not attempt to fetch the object if missing (even if fetch_is_missing is - * nonzero). - */ -#define OBJECT_INFO_SKIP_FETCH_OBJECT (1 << 2) -/* - * 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), -/* Die if object corruption (not just an object being missing) was detected. */ -#define OBJECT_INFO_DIE_IF_CORRUPT (1 << 3) + /* 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), + + /* + * 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` @@ -377,7 +383,7 @@ struct object_info { int odb_read_object_info_extended(struct object_database *odb, const struct object_id *oid, struct object_info *oi, - unsigned flags); + enum object_info_flags flags); /* * Read a subset of object info for the given object ID. Returns an `enum diff --git a/packfile.c b/packfile.c index 402c3b5dc7..cb418846ae 100644 --- a/packfile.c +++ b/packfile.c @@ -2149,7 +2149,7 @@ int packfile_store_freshen_object(struct packfile_store *store, int packfile_store_read_object_info(struct packfile_store *store, const struct object_id *oid, struct object_info *oi, - unsigned flags UNUSED) + enum object_info_flags flags UNUSED) { struct pack_entry e; int ret; diff --git a/packfile.h b/packfile.h index acc5c55ad5..989fd10cb6 100644 --- a/packfile.h +++ b/packfile.h @@ -247,7 +247,7 @@ int packfile_store_read_object_stream(struct odb_read_stream **out, int packfile_store_read_object_info(struct packfile_store *store, const struct object_id *oid, struct object_info *oi, - unsigned flags); + enum object_info_flags flags); /* * Open the packfile and add it to the store if it isn't yet known. Returns From 732ec9b17b78a49496bfb796fcfe606f3a9f02f1 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 12 Feb 2026 07:59:41 +0100 Subject: [PATCH 5/5] odb: convert `odb_has_object()` flags into an enum Following the reason in the preceding commit, convert the `odb_has_object()` flags into an enum. With this change, we would have catched the misuse of `odb_has_object()` that was fixed in a preceding commit as the compiler would have generated a warning: ../builtin/backfill.c:71:9: error: implicit conversion from enumeration type 'enum odb_object_info_flag' to different enumeration type 'enum odb_has_object_flag' [-Werror,-Wenum-conversion] 70 | if (!odb_has_object(ctx->repo->objects, &list->oid[i], | ~~~~~~~~~~~~~~ 71 | OBJECT_INFO_FOR_PREFETCH)) | ^~~~~~~~~~~~~~~~~~~~~~~~ 1 error generated. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- odb.c | 2 +- odb.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/odb.c b/odb.c index d437aa8b06..2bbbfb344a 100644 --- a/odb.c +++ b/odb.c @@ -964,7 +964,7 @@ void *odb_read_object_peeled(struct object_database *odb, } int odb_has_object(struct object_database *odb, const struct object_id *oid, - unsigned flags) + enum has_object_flags flags) { unsigned object_info_flags = 0; diff --git a/odb.h b/odb.h index e94cdc3665..f7368827ac 100644 --- a/odb.h +++ b/odb.h @@ -395,7 +395,7 @@ int odb_read_object_info(struct object_database *odb, const struct object_id *oid, unsigned long *sizep); -enum { +enum has_object_flags { /* Retry packed storage after checking packed and loose storage */ HAS_OBJECT_RECHECK_PACKED = (1 << 0), /* Allow fetching the object in case the repository has a promisor remote. */ @@ -408,7 +408,7 @@ enum { */ int odb_has_object(struct object_database *odb, const struct object_id *oid, - unsigned flags); + enum has_object_flags flags); int odb_freshen_object(struct object_database *odb, const struct object_id *oid);