{"thread":{"id":"66194","subject":"[PATCH v2 1/5] odb/source-packed: flag known-bad objects as corrupt and not missing","startedAt":"2026-08-19T12:17:52Z","lastAt":"2026-08-21T12:39:56Z","messageCount":15,"participants":["Patrick Steinhardt","Karthik Nayak"],"isPatch":true,"patchVersion":2,"patchTotal":5},"messages":[{"id":"550804","messageId":"20260819-pks-odb-generic-corrupt-objects-v2-0-a984e3a0ad6f@pks.im","threadId":"66194","inReplyTo":"20260818-pks-odb-generic-corrupt-objects-v1-0-ec234567510f@pks.im","subject":"[PATCH v2 0/5] odb: handle `OBJECT_INFO_DIE_IF_CORRUPT` generically","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-19T12:17:18Z","receivedAt":"2026-08-19T12:17:51Z","isPatch":true,"body":"Hi,\n\nwhen looking up an object with `OBJECT_INFO_DIE_IF_CORRUPT` fails we\nwant to die in case the object exists but is corrupted. This flag is\nhandled in two different spots right now:\n\n - `do_oid_object_info_extended()` calls `has_packed_and_bad()` to\n check whether the object is known to be corrupt in any packfile.\n This function reaches into the internals of the packed source and\n thus breaks the abstraction provided by our object sources.\n\n - The loose source handles the flag itself and dies directly in\n `read_object_info_from_path()`, which means that we die even in\n cases where another source may still have a good copy of the\n object.\n\nBesides being inconsistent, it also ties us to the specific backend used\nby the database sources because `has_packed_and_bad()` assumes that they\nuse the \"files\" backend. Any other backend will instead cause us to die\nwhen calling `odb_source_files_downcast()`, even if the object was\nsimply nonexistent.\n\nThis series fixes these issues and makes the check backend-agnostic by\nextending semantics of `odb_source_read_object_info()`: on the one hand\nit now distinguishes whether an object is missing or corrput, and on the\nother hand it starts to return an error message to the caller.\n\nChanges in v2:\n - Adapt the series to use an `enum odb_read_status` with negative\n error codes exclusively, as suggested by Junio. This results in a\n rather big restructure of the series.\n - Link to v1: https://patch.msgid.link/20260818-pks-odb-generic-corrupt-objects-v1-0-ec234567510f@pks.im\n\nThanks!\n\nPatrick\n\n---\nPatrick Steinhardt (5):\n odb/source-packed: flag known-bad objects as corrupt and not missing\n odb/source: introduce error status when reading objects\n odb/source: let callers discern missing and corrupt objects\n odb/source: allow `read_object_info()` to bubble up error messages\n odb: handle `OBJECT_INFO_DIE_IF_CORRUPT` generically\n\n builtin/pack-objects.c | 8 +++---\n midx.c | 10 ++++---\n midx.h | 3 ++-\n odb.c | 63 ++++++++++++++++++++++++++++---------------\n odb.h | 17 +++++++++---\n odb/source-files.c | 31 ++++++++++++++++-----\n odb/source-inmemory.c | 11 ++++----\n odb/source-loose.c | 52 ++++++++++++++++++++---------------\n odb/source-packed.c | 58 ++++++++++++++++++++++++++++-----------\n odb/source.h | 34 ++++++++++++++---------\n packfile.c | 29 ++++++--------------\n packfile.h | 4 +--\n t/helper/test-read-midx.c | 2 +-\n t/t1060-object-corruption.sh | 18 +++++++++++++\n t/unit-tests/u-odb-inmemory.c | 5 ++--\n 15 files changed, 224 insertions(+), 121 deletions(-)\n\nRange-diff versus v1:\n\n1: ea7d64242a < -: ---------- odb/source: discern missing and corrupt objects\n2: f4944112b3 < -: ---------- odb/source-inmemory: signal missing objects via positive return\n3: 9080d7f138 ! 1: c821c3b004 odb/source-packed: flag known-bad objects as corrupt and not missing\n @@ Metadata\n ## Commit message ##\n odb/source-packed: flag known-bad objects as corrupt and not missing\n \n - When reading a packed object that doesn't verify we mark it as bad and\n - indicate to the caller that we failed reading the object despite the\n - fact that it supposedly exists. This matches the semantics we have now\n - established in a preceding commit, where we discern failure to read a\n - corrupt object from a missing object.\n + When reading packed objects we know to tell apart missing objects and\n + corrupt objects by returning a positive error code in the former case,\n + and a negative one in the latter case. We do that by distinguishing\n + between errors returned by `find_pack_entry()`, which yields the offset\n + of the object, and `packed_object_info()`, which reads the object\n + contents.\n \n - What doesn't work yet though is when a call tries to read an object that\n - has already been marked as corrupt in a previous call. In that case,\n - `find_pack_entry()` will tell us that the object in question does not\n - exist, and consequently we'll not flag the object as corrupt but as\n - missing.\n + But even though we already distinguish those cases when reading packed\n + objects, the logic is broken in case a caller tries to read an object\n + that has been marked as corrupt. In that case, `find_pack_entry()` will\n + tell us that the object in question does not exist, and consequently\n + we'll not flag the object as corrupt but as missing.\n \n Fix this issue by bubbling up whether the object is corrupt and, if so,\n - which packfile contains the corrupted object. We don't yet need the\n - latter information about the specific packfile, so we could've just as\n - well made this a `bool *corrupted` pointer. But we'll need information\n - about the containing packfile in a subsequent commit.\n + which packfile contains the corrupted object.\n +\n + Note that we don't yet need the information about the specific packfile,\n + so we could've just as well made this a `bool *corrupted` pointer. But\n + we'll need information about the containing packfile in a subsequent\n + commit so that we can generate a proper error message telling the user\n + which packfile contains the broken object.\n \n Signed-off-by: Patrick Steinhardt \n \n @@ odb/source-packed.c: static int odb_source_packed_read_object_info(struct odb_so\n -\tif (!find_pack_entry(packed, oid, &e))\n +\tif (!find_pack_entry(packed, oid, &e, &bad_pack)) {\n +\t\t/*\n -+\t\t * The lookup may have failed because the object is known to\n -+\t\t * be corrupt in one of our packfiles, in which case the\n -+\t\t * corresponding pack entries are skipped. Report the object\n -+\t\t * as corrupt instead of as missing in that case.\n ++\t\t * The lookup may have failed because the object is known to be\n ++\t\t * corrupt in one of the packfiles. Report the object as\n ++\t\t * corrupt instead of missing in that case.\n +\t\t */\n +\t\tif (bad_pack)\n +\t\t\treturn -1;\n-: ---------- > 2: 602249a58e odb/source: introduce error status when reading objects\n4: db2bd77c61 ! 3: 269fb8e6a6 odb/source-loose: distinguish missing and corrupt objects\n @@ Metadata\n Author: Patrick Steinhardt \n \n ## Commit message ##\n - odb/source-loose: distinguish missing and corrupt objects\n + odb/source: let callers discern missing and corrupt objects\n \n - The loose source returns a negative value from its `read_object_info()`\n - callback both when the object is missing and when the object exists but\n - cannot be read. Consequently, callers cannot tell apart whether the\n - object does not exist in this source at all or whether it is corrupt.\n + As explained in the preceding commits, reading objects can either fail\n + because the object truly does not exist or because it exists, but its\n + data is corrupt. Some callers do care about this distinction, but there\n + is no way to tell these two cases apart right now.\n \n - Adapt the code to return a positive value for missing objects according\n - to the new calling convention.\n + Introduce a new `ODB_READ_NOT_FOUND` value that ought to be returned by\n + the backends in case the object truly does not exist and adapt backends\n + to use it.\n \n - This also allows us to get rid of the separate `corrupt:` label, as we\n - can now clearly distinguish between corrupt and missing objects in the\n - function ourselves. This makes us handle failures to read loose objects\n - more consistently, as not all failure cases were jumping that label.\n -\n - Note that there's one call to `die()` when the object type is invalid\n - that should arguably be converted to an error, too. But adapting that\n - call results in quite a lot of broken tests, so this is left as-is for\n - now.\n + Note that we don't yet return this error from `odb_read_object_info()`\n + itself. This will be fixed in a subsequent commit.\n \n Signed-off-by: Patrick Steinhardt \n \n + ## odb.h ##\n +@@ odb.h: enum odb_read_status {\n + \tODB_READ_OK = 0,\n + \t/* The read resulted in a generic error. */\n + \tODB_READ_ERROR = -1,\n ++\t/* The object could not be found. */\n ++\tODB_READ_NOT_FOUND = -2,\n + };\n + \n + /*\n +\n + ## odb/source-files.c ##\n +@@ odb/source-files.c: static enum odb_read_status odb_source_files_read_object_info(struct odb_source\n + \t\t\t\t\t\t\t enum object_info_flags flags)\n + {\n + \tstruct odb_source_files *files = odb_source_files_downcast(source);\n ++\tenum odb_read_status ret_packed, ret_loose;\n + \n +-\tif (!odb_source_read_object_info(&files->packed->base, oid, oi, flags) ||\n +-\t !odb_source_read_object_info(&files->loose->base, oid, oi, flags))\n ++\tret_packed = odb_source_read_object_info(&files->packed->base, oid, oi, flags);\n ++\tif (!ret_packed)\n + \t\treturn 0;\n + \n +-\treturn -1;\n ++\tret_loose = odb_source_read_object_info(&files->loose->base, oid, oi, flags);\n ++\tif (!ret_loose)\n ++\t\treturn 0;\n ++\n ++\t/*\n ++\t * Reading the packed object may have failed even though the object\n ++\t * exists, for example because it is corrupt. Report this failure to\n ++\t * the caller in case neither of the sources was able to read the\n ++\t * object, and prefer the error of the packed source in case both\n ++\t * reads have failed.\n ++\t */\n ++\tif (ret_packed != ODB_READ_NOT_FOUND)\n ++\t\treturn ret_packed;\n ++\treturn ret_loose;\n + }\n + \n + static int odb_source_files_read_object_stream(struct odb_read_stream **out,\n +\n + ## odb/source-inmemory.c ##\n +@@ odb/source-inmemory.c: static enum odb_read_status odb_source_inmemory_read_object_info(struct odb_sour\n + \n + \tobject = find_cached_object(inmemory, oid);\n + \tif (!object)\n +-\t\treturn -1;\n ++\t\treturn ODB_READ_NOT_FOUND;\n + \n + \tpopulate_object_info(inmemory, oi, object);\n + \treturn 0;\n +\n ## odb/source-loose.c ##\n @@ odb/source-loose.c: static int read_object_info_from_path(struct odb_source_loose *loose,\n \t\tstruct stat st;\n \n \t\tif ((!oi || (!oi->disk_sizep && !oi->mtimep)) && (flags & OBJECT_INFO_QUICK)) {\n -\t\t\tret = quick_has_loose(loose, oid) ? 0 : -1;\n -+\t\t\tret = quick_has_loose(loose, oid) ? 0 : 1;\n ++\t\t\tret = quick_has_loose(loose, oid) ? 0 : ODB_READ_NOT_FOUND;\n \t\t\tgoto out;\n \t\t}\n \n \t\tif (lstat(path, &st) < 0) {\n +\t\t\tif (errno == ENOENT) {\n -+\t\t\t\tret = 1;\n ++\t\t\t\tret = ODB_READ_NOT_FOUND;\n +\t\t\t\tgoto out;\n +\t\t\t}\n +\n @@ odb/source-loose.c: static int read_object_info_from_path(struct odb_source_loos\n -\t\t\terror_errno(_(\"unable to open loose object %s\"), oid_to_hex(oid));\n -\t\tret = -1;\n +\t\tif (errno == ENOENT) {\n -+\t\t\tret = 1;\n ++\t\t\tret = ODB_READ_NOT_FOUND;\n +\t\t\tgoto out;\n +\t\t}\n +\n @@ odb/source-loose.c: static int read_object_info_from_path(struct odb_source_loos\n -corrupt:\n -\tif (ret && (flags & OBJECT_INFO_DIE_IF_CORRUPT))\n +out:\n -+\tif (ret < 0 && (flags & OBJECT_INFO_DIE_IF_CORRUPT))\n ++\tif (ret && ret != ODB_READ_NOT_FOUND && (flags & OBJECT_INFO_DIE_IF_CORRUPT))\n \t\tdie(_(\"loose object %s (stored in %s) is corrupt\"),\n \t\t oid_to_hex(oid), path);\n \n @@ odb/source-loose.c: static int read_object_info_from_path(struct odb_source_loos\n \tif (stream_to_end)\n \t\tgit_inflate_end(stream_to_end);\n \tif (map)\n -@@ odb/source-loose.c: static int odb_source_loose_read_object_info(struct odb_source *source,\n +@@ odb/source-loose.c: static enum odb_read_status odb_source_loose_read_object_info(struct odb_source\n \t * second time.\n \t */\n \tif (flags & OBJECT_INFO_SECOND_READ)\n -\t\treturn -1;\n -+\t\treturn 1;\n ++\t\treturn ODB_READ_NOT_FOUND;\n \n \todb_loose_path(loose, &buf, oid);\n \treturn read_object_info_from_path(loose, buf.buf, oid, oi, flags);\n -@@ odb/source-loose.c: static int for_each_object_wrapper_cb(const struct object_id *oid,\n - \tif (data->request) {\n - \t\tstruct object_info oi = *data->request;\n - \n --\t\tif (read_object_info_from_path(data->loose, path, oid, &oi, 0) < 0)\n -+\t\tif (read_object_info_from_path(data->loose, path, oid, &oi, 0))\n +\n + ## odb/source-packed.c ##\n +@@ odb/source-packed.c: static enum odb_read_status odb_source_packed_read_object_info(struct odb_source\n + \t\t */\n + \t\tif (bad_pack)\n \t\t\treturn -1;\n +-\t\treturn 1;\n ++\t\treturn ODB_READ_NOT_FOUND;\n + \t}\n \n - \t\treturn data->cb(oid, &oi, data->cb_data);\n -@@ odb/source-loose.c: static int for_each_prefixed_object_wrapper_cb(const struct object_id *oid,\n - \t\tstruct object_info oi = *data->request;\n + \t/*\n +\n + ## t/unit-tests/u-odb-inmemory.c ##\n +@@ t/unit-tests/u-odb-inmemory.c: void test_odb_inmemory__read_missing_object(void)\n + \tconst char *end;\n \n - \t\tif (odb_source_read_object_info(&data->loose->base,\n --\t\t\t\t\t\toid, &oi, 0) < 0)\n -+\t\t\t\t\t\toid, &oi, 0))\n - \t\t\treturn -1;\n + \tcl_must_pass(parse_oid_hex_algop(RANDOM_OID, &oid, &end, repo.hash_algo));\n +-\tcl_must_fail(odb_source_read_object_info(&source->base, &oid, NULL, 0));\n ++\tcl_assert_equal_i(odb_source_read_object_info(&source->base, &oid, NULL, 0),\n ++\t\t\t ODB_READ_NOT_FOUND);\n \n - \t\treturn data->cb(oid, &oi, data->cb_data);\n + \todb_source_free(&source->base);\n + }\n5: 0c9be02ab4 < -: ---------- odb/source-files: signal mark objects via positive return\n6: f633262bd4 ! 4: 7de629151d odb/source: allow `read_object_info()` to bubble up error messages\n @@ builtin/pack-objects.c: static int force_object_loose(struct odb_source *source,\n \n \n ## odb.c ##\n -@@ odb.c: static int do_oid_object_info_extended(struct object_database *odb,\n +@@ odb.c: static enum odb_read_status do_oid_object_info_extended(struct object_database *\n \tif (is_null_oid(real))\n \t\treturn -1;\n \n @@ odb.c: static int do_oid_object_info_extended(struct object_database *odb,\n \t\treturn 0;\n \n \todb_prepare_alternates(odb);\n -@@ odb.c: static int do_oid_object_info_extended(struct object_database *odb,\n +@@ odb.c: static enum odb_read_status do_oid_object_info_extended(struct object_database *\n \t\tstruct odb_source *source;\n \n \t\tfor (source = odb->sources; source; source = source->next)\n @@ odb.c: static int do_oid_object_info_extended(struct object_database *odb,\n \t\t\t\treturn 0;\n \n \t\t/*\n -@@ odb.c: static int do_oid_object_info_extended(struct object_database *odb,\n +@@ odb.c: static enum odb_read_status do_oid_object_info_extended(struct object_database *\n \t\tif (!(flags & OBJECT_INFO_QUICK)) {\n \t\t\tfor (source = odb->sources; source; source = source->next)\n \t\t\t\tif (!odb_source_read_object_info(source, real, oi,\n @@ odb.c: static int do_oid_object_info_extended(struct object_database *odb,\n \n ## odb/source-files.c ##\n @@ odb/source-files.c: static void odb_source_files_prepare(struct odb_source *source,\n - static int odb_source_files_read_object_info(struct odb_source *source,\n - \t\t\t\t\t const struct object_id *oid,\n - \t\t\t\t\t struct object_info *oi,\n --\t\t\t\t\t enum object_info_flags flags)\n -+\t\t\t\t\t enum object_info_flags flags,\n -+\t\t\t\t\t struct strbuf *errmsg)\n + static enum odb_read_status odb_source_files_read_object_info(struct odb_source *source,\n + \t\t\t\t\t\t\t const struct object_id *oid,\n + \t\t\t\t\t\t\t struct object_info *oi,\n +-\t\t\t\t\t\t\t enum object_info_flags flags)\n ++\t\t\t\t\t\t\t enum object_info_flags flags,\n ++\t\t\t\t\t\t\t struct strbuf *errmsg)\n {\n \tstruct odb_source_files *files = odb_source_files_downcast(source);\n - \tint ret_packed, ret_loose;\n + \tenum odb_read_status ret_packed, ret_loose;\n \n -\tret_packed = odb_source_read_object_info(&files->packed->base, oid, oi, flags);\n +\tret_packed = odb_source_read_object_info(&files->packed->base, oid, oi,\n @@ odb/source-files.c: static void odb_source_files_prepare(struct odb_source *sour\n \t\treturn 0;\n \n -\tret_loose = odb_source_read_object_info(&files->loose->base, oid, oi, flags);\n -+\tret_loose = odb_source_read_object_info(&files->loose->base, oid, oi,\n -+\t\t\t\t\t\t flags, ret_packed < 0 ? NULL : errmsg);\n ++\tret_loose = odb_source_read_object_info(&files->loose->base, oid, oi, flags,\n ++\t\t\t\t\t\tret_packed == ODB_READ_NOT_FOUND ? errmsg : NULL);\n \tif (!ret_loose)\n \t\treturn 0;\n \n \n ## odb/source-inmemory.c ##\n @@ odb/source-inmemory.c: static void populate_object_info(struct odb_source_inmemory *source,\n - static int odb_source_inmemory_read_object_info(struct odb_source *source,\n - \t\t\t\t\t\tconst struct object_id *oid,\n - \t\t\t\t\t\tstruct object_info *oi,\n --\t\t\t\t\t\tenum object_info_flags flags UNUSED)\n -+\t\t\t\t\t\tenum object_info_flags flags UNUSED,\n -+\t\t\t\t\t\tstruct strbuf *errmsg UNUSED)\n + static enum odb_read_status odb_source_inmemory_read_object_info(struct odb_source *source,\n + \t\t\t\t\t\t\t\t const struct object_id *oid,\n + \t\t\t\t\t\t\t\t struct object_info *oi,\n +-\t\t\t\t\t\t\t\t enum object_info_flags flags UNUSED)\n ++\t\t\t\t\t\t\t\t enum object_info_flags flags UNUSED,\n ++\t\t\t\t\t\t\t\t struct strbuf *errmsg UNUSED)\n {\n \tstruct odb_source_inmemory *inmemory = odb_source_inmemory_downcast(source);\n \tconst struct inmemory_object *object;\n @@ odb/source-loose.c: static int read_object_info_from_path(struct odb_source_loos\n \tret = 0;\n \n out:\n -+\tif (ret < 0 && errmsg)\n -+\t\tstrbuf_addf(errmsg, _(\"loose object %s (stored in %s) is corrupt\"),\n +-\tif (ret && ret != ODB_READ_NOT_FOUND && (flags & OBJECT_INFO_DIE_IF_CORRUPT))\n +-\t\tdie(_(\"loose object %s (stored in %s) is corrupt\"),\n +-\t\t oid_to_hex(oid), path);\n ++\tif (ret && ret != ODB_READ_NOT_FOUND) {\n ++\t\tif ((flags & OBJECT_INFO_DIE_IF_CORRUPT))\n ++\t\t\tdie(_(\"loose object %s (stored in %s) is corrupt\"),\n +\t\t\t oid_to_hex(oid), path);\n -+\n - \tif (ret < 0 && (flags & OBJECT_INFO_DIE_IF_CORRUPT))\n - \t\tdie(_(\"loose object %s (stored in %s) is corrupt\"),\n - \t\t oid_to_hex(oid), path);\n ++\t\tif (errmsg)\n ++\t\t\tstrbuf_addf(errmsg, _(\"loose object %s (stored in %s) is corrupt\"),\n ++\t\t\t\t oid_to_hex(oid), path);\n ++\t}\n + \n + \tif (stream_to_end)\n + \t\tgit_inflate_end(stream_to_end);\n @@ odb/source-loose.c: static int read_object_info_from_path(struct odb_source_loose *loose,\n - static int odb_source_loose_read_object_info(struct odb_source *source,\n - \t\t\t\t\t const struct object_id *oid,\n - \t\t\t\t\t struct object_info *oi,\n --\t\t\t\t\t enum object_info_flags flags)\n -+\t\t\t\t\t enum object_info_flags flags,\n -+\t\t\t\t\t struct strbuf *errmsg)\n + static enum odb_read_status odb_source_loose_read_object_info(struct odb_source *source,\n + \t\t\t\t\t\t\t const struct object_id *oid,\n + \t\t\t\t\t\t\t struct object_info *oi,\n +-\t\t\t\t\t\t\t enum object_info_flags flags)\n ++\t\t\t\t\t\t\t enum object_info_flags flags,\n ++\t\t\t\t\t\t\t struct strbuf *errmsg)\n {\n \tstruct odb_source_loose *loose = odb_source_loose_downcast(source);\n \tstatic struct strbuf buf = STRBUF_INIT;\n -@@ odb/source-loose.c: static int odb_source_loose_read_object_info(struct odb_source *source,\n - \t\treturn 1;\n +@@ odb/source-loose.c: static enum odb_read_status odb_source_loose_read_object_info(struct odb_source\n + \t\treturn ODB_READ_NOT_FOUND;\n \n \todb_loose_path(loose, &buf, oid);\n -\treturn read_object_info_from_path(loose, buf.buf, oid, oi, flags);\n @@ odb/source-loose.c: static int for_each_object_wrapper_cb(const struct object_id\n \tif (data->request) {\n \t\tstruct object_info oi = *data->request;\n \n --\t\tif (read_object_info_from_path(data->loose, path, oid, &oi, 0))\n -+\t\tif (read_object_info_from_path(data->loose, path, oid, &oi, 0, NULL))\n +-\t\tif (read_object_info_from_path(data->loose, path, oid, &oi, 0) < 0)\n ++\t\tif (read_object_info_from_path(data->loose, path, oid, &oi, 0, NULL) < 0)\n \t\t\treturn -1;\n \n \t\treturn data->cb(oid, &oi, data->cb_data);\n @@ odb/source-loose.c: static int for_each_prefixed_object_wrapper_cb(const struct\n \t\tstruct object_info oi = *data->request;\n \n \t\tif (odb_source_read_object_info(&data->loose->base,\n --\t\t\t\t\t\toid, &oi, 0))\n -+\t\t\t\t\t\toid, &oi, 0, NULL))\n +-\t\t\t\t\t\toid, &oi, 0) < 0)\n ++\t\t\t\t\t\toid, &oi, 0, NULL) < 0)\n \t\t\treturn -1;\n \n \t\treturn data->cb(oid, &oi, data->cb_data);\n @@ odb/source-packed.c\n static int find_pack_entry(struct odb_source_packed *store,\n \t\t\t const struct object_id *oid,\n @@ odb/source-packed.c: static int find_pack_entry(struct odb_source_packed *store,\n - static int odb_source_packed_read_object_info(struct odb_source *source,\n - \t\t\t\t\t const struct object_id *oid,\n - \t\t\t\t\t struct object_info *oi,\n --\t\t\t\t\t enum object_info_flags flags)\n -+\t\t\t\t\t enum object_info_flags flags,\n -+\t\t\t\t\t struct strbuf *errmsg)\n + static enum odb_read_status odb_source_packed_read_object_info(struct odb_source *source,\n + \t\t\t\t\t\t\t const struct object_id *oid,\n + \t\t\t\t\t\t\t struct object_info *oi,\n +-\t\t\t\t\t\t\t enum object_info_flags flags)\n ++\t\t\t\t\t\t\t enum object_info_flags flags,\n ++\t\t\t\t\t\t\t struct strbuf *errmsg)\n {\n \tstruct odb_source_packed *packed = odb_source_packed_downcast(source);\n \tstruct packed_git *bad_pack = NULL;\n -@@ odb/source-packed.c: static int odb_source_packed_read_object_info(struct odb_source *source,\n - \t\t * corresponding pack entries are skipped. Report the object\n - \t\t * as corrupt instead of as missing in that case.\n +@@ odb/source-packed.c: static enum odb_read_status odb_source_packed_read_object_info(struct odb_source\n + \t\t * corrupt in one of the packfiles. Report the object as\n + \t\t * corrupt instead of missing in that case.\n \t\t */\n -\t\tif (bad_pack)\n -\t\t\treturn -1;\n --\t\treturn 1;\n +-\t\treturn ODB_READ_NOT_FOUND;\n +\t\tif (bad_pack) {\n +\t\t\tret = -1;\n +\t\t\tgoto out;\n +\t\t}\n +\n -+\t\tret = 1;\n ++\t\tret = ODB_READ_NOT_FOUND;\n +\t\tgoto out;\n \t}\n \n @@ odb/source-packed.c: static int odb_source_packed_read_object_info(struct odb_so\n +\tret = 0;\n +\n +out:\n -+\tif (bad_pack && errmsg)\n ++\tif (ret < 0 && bad_pack && errmsg)\n +\t\tstrbuf_addf(errmsg, _(\"packed object %s (stored in %s) is corrupt\"),\n +\t\t\t oid_to_hex(oid), bad_pack->pack_name);\n +\n @@ odb/source.h: enum odb_source_type {\n \n /*\n @@ odb/source.h: struct odb_source {\n - \t * - A negative value in case the object exists in this source, but\n - \t * reading its object info has failed, for example because its\n - \t * on-disk state is corrupt.\n -+\t *\n -+\t * In case reading the object has failed and `errmsg` is non-NULL, the\n -+\t * callback is expected to populate it with a human-readable message\n -+\t * that describes the failure.\n + \t * already surfaced the object without reloading any on-disk state.\n + \t *\n + \t * The callback is expected to return an `enum odb_read_status`. Please\n +-\t * refer to the individual values that can be returned.\n ++\t * refer to the individual values that can be returned. In case reading\n ++\t * the object has failed with a generic error and `errmsg` is non-NULL,\n ++\t * the callback is expected to populate it with a human-readable\n ++\t * message that describes the failure.\n \t */\n - \tint (*read_object_info)(struct odb_source *source,\n - \t\t\t\tconst struct object_id *oid,\n - \t\t\t\tstruct object_info *oi,\n --\t\t\t\tenum object_info_flags flags);\n -+\t\t\t\tenum object_info_flags flags,\n -+\t\t\t\tstruct strbuf *errmsg);\n + \tenum odb_read_status (*read_object_info)(struct odb_source *source,\n + \t\t\t\t\t\t const struct object_id *oid,\n + \t\t\t\t\t\t struct object_info *oi,\n +-\t\t\t\t\t\t enum object_info_flags flags);\n ++\t\t\t\t\t\t enum object_info_flags flags,\n ++\t\t\t\t\t\t struct strbuf *errmsg);\n \n \t/*\n \t * This callback is expected to create a new read stream that can be\n @@ odb/source.h: static inline void odb_source_prepare(struct odb_source *source,\n - * Returns 0 on success, a positive value in case the object is missing in the\n - * source and a negative value in case the object exists, but reading it has\n - * failed.\n + /*\n + * Read an object from the object database source identified by its object ID.\n + * Please refer to `enum odb_read_status` for the individual error codes.\n + *\n -+ * In case reading the object has failed and `errmsg` is non-NULL it will be\n -+ * populated with a human-readable message that describes the failure.\n ++ * In case reading the object has failed with a generic error and `errmsg` is\n ++ * non-NULL it will be populated with a human-readable message that describes\n ++ * the failure.\n */\n - static inline int odb_source_read_object_info(struct odb_source *source,\n - \t\t\t\t\t const struct object_id *oid,\n - \t\t\t\t\t struct object_info *oi,\n --\t\t\t\t\t enum object_info_flags flags)\n -+\t\t\t\t\t enum object_info_flags flags,\n -+\t\t\t\t\t struct strbuf *errmsg)\n + static inline enum odb_read_status odb_source_read_object_info(struct odb_source *source,\n + \t\t\t\t\t\t\t const struct object_id *oid,\n + \t\t\t\t\t\t\t struct object_info *oi,\n +-\t\t\t\t\t\t\t enum object_info_flags flags)\n ++\t\t\t\t\t\t\t enum object_info_flags flags,\n ++\t\t\t\t\t\t\t struct strbuf *errmsg)\n {\n -\treturn source->read_object_info(source, oid, oi, flags);\n +\treturn source->read_object_info(source, oid, oi, flags, errmsg);\n @@ t/unit-tests/u-odb-inmemory.c: void test_odb_inmemory__read_missing_object(void)\n \tconst char *end;\n \n \tcl_must_pass(parse_oid_hex_algop(RANDOM_OID, &oid, &end, repo.hash_algo));\n --\tcl_assert(odb_source_read_object_info(&source->base, &oid, NULL, 0) > 0);\n -+\tcl_assert(odb_source_read_object_info(&source->base, &oid, NULL, 0, NULL) > 0);\n +-\tcl_assert_equal_i(odb_source_read_object_info(&source->base, &oid, NULL, 0),\n ++\tcl_assert_equal_i(odb_source_read_object_info(&source->base, &oid, NULL, 0, NULL),\n + \t\t\t ODB_READ_NOT_FOUND);\n \n \todb_source_free(&source->base);\n - }\n7: a82f4341e1 ! 5: 4af62fe3bf odb: handle `OBJECT_INFO_DIE_IF_CORRUPT` generically\n @@ Commit message\n In the preceding commits we've carved out the infrastructure to make\n this mechanism fully generic. On the one hand, all backends now tell us\n whether the object is missing or corrupt via their return values. And\n - on the other hand, they have been tought to provide a readable error\n + on the other hand, they have been taught to provide a readable error\n message to the caller.\n \n Adapt `do_oid_object_info_extended()` to use those new mechanisms. This\n @@ odb.c\n #include \"path.h\"\n #include \"promisor-remote.h\"\n #include \"quote.h\"\n -@@ odb.c: static int do_oid_object_info_extended(struct object_database *odb,\n - \t\t\t\t const struct object_id *oid,\n - \t\t\t\t struct object_info *oi, unsigned flags)\n +@@ odb.c: static enum odb_read_status do_oid_object_info_extended(struct object_database *\n + \t\t\t\t\t\t\tconst struct object_id *oid,\n + \t\t\t\t\t\t\tstruct object_info *oi, unsigned flags)\n {\n +\tstruct strbuf corrupt_err = STRBUF_INIT;\n \tconst struct object_id *real = oid;\n ++\tenum odb_read_status ret;\n \tint already_retried = 0;\n +\tbool corrupt = false;\n -+\tint ret;\n \n \tif (flags & OBJECT_INFO_LOOKUP_REPLACE)\n \t\treal = lookup_replace_object(odb->repo, oid);\n -@@ odb.c: static int do_oid_object_info_extended(struct object_database *odb,\n +@@ odb.c: static enum odb_read_status do_oid_object_info_extended(struct object_database *\n \twhile (1) {\n \t\tstruct odb_source *source;\n \n @@ odb.c: static int do_oid_object_info_extended(struct object_database *odb,\n +\t\t\t\t\t\t\t corrupt_err.len ? NULL : &corrupt_err);\n +\t\t\tif (!ret)\n +\t\t\t\tgoto out;\n -+\t\t\tif (ret < 0)\n ++\t\t\tif (ret != ODB_READ_NOT_FOUND)\n +\t\t\t\tcorrupt = true;\n +\t\t}\n \n \t\t/*\n \t\t * When the object hasn't been found we try a second read and\n -@@ odb.c: static int do_oid_object_info_extended(struct object_database *odb,\n +@@ odb.c: static enum odb_read_status do_oid_object_info_extended(struct object_database *\n \t\t * caches or reload on-disk state.\n \t\t */\n \t\tif (!(flags & OBJECT_INFO_QUICK)) {\n @@ odb.c: static int do_oid_object_info_extended(struct object_database *odb,\n +\t\t\t\t\t\t\t\t corrupt_err.len ? NULL : &corrupt_err);\n +\t\t\t\tif (!ret)\n +\t\t\t\t\tgoto out;\n -+\t\t\t\tif (ret < 0)\n ++\t\t\t\tif (ret != ODB_READ_NOT_FOUND)\n +\t\t\t\t\tcorrupt = true;\n +\t\t\t}\n \t\t}\n \n \t\t/*\n -@@ odb.c: static int do_oid_object_info_extended(struct object_database *odb,\n +@@ odb.c: static enum odb_read_status do_oid_object_info_extended(struct object_database *\n \t\t}\n \n \t\tif (flags & OBJECT_INFO_DIE_IF_CORRUPT) {\n @@ odb.c: static int do_oid_object_info_extended(struct object_database *odb,\n \t\t}\n -\t\treturn -1;\n +\n -+\t\tret = -1;\n ++\t\tret = corrupt ? ODB_READ_ERROR : ODB_READ_NOT_FOUND;\n +\t\tgoto out;\n \t}\n +\n @@ odb.c: static int do_oid_object_info_extended(struct object_database *odb,\n \n ## odb/source-loose.c ##\n @@ odb/source-loose.c: static int read_object_info_from_path(struct odb_source_loose *loose,\n - \tif (ret < 0 && errmsg)\n - \t\tstrbuf_addf(errmsg, _(\"loose object %s (stored in %s) is corrupt\"),\n + \tret = 0;\n + \n + out:\n +-\tif (ret && ret != ODB_READ_NOT_FOUND) {\n +-\t\tif ((flags & OBJECT_INFO_DIE_IF_CORRUPT))\n +-\t\t\tdie(_(\"loose object %s (stored in %s) is corrupt\"),\n ++\tif (ret && ret != ODB_READ_NOT_FOUND && errmsg)\n ++\t\tstrbuf_addf(errmsg, _(\"loose object %s (stored in %s) is corrupt\"),\n \t\t\t oid_to_hex(oid), path);\n --\n --\tif (ret < 0 && (flags & OBJECT_INFO_DIE_IF_CORRUPT))\n --\t\tdie(_(\"loose object %s (stored in %s) is corrupt\"),\n --\t\t oid_to_hex(oid), path);\n +-\t\tif (errmsg)\n +-\t\t\tstrbuf_addf(errmsg, _(\"loose object %s (stored in %s) is corrupt\"),\n +-\t\t\t\t oid_to_hex(oid), path);\n +-\t}\n -\n \tif (stream_to_end)\n \t\tgit_inflate_end(stream_to_end);\n\n---\nbase-commit: 18e66859d87fb4b76599f73460b54f0848c76b16\nchange-id: 20260818-pks-odb-generic-corrupt-objects-52a47d6214d9\n\n"},{"id":"550803","messageId":"20260819-pks-odb-generic-corrupt-objects-v2-1-a984e3a0ad6f@pks.im","threadId":"66194","inReplyTo":"20260819-pks-odb-generic-corrupt-objects-v2-0-a984e3a0ad6f@pks.im","subject":"[PATCH v2 1/5] odb/source-packed: flag known-bad objects as corrupt and not missing","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-19T12:17:19Z","receivedAt":"2026-08-19T12:17:52Z","isPatch":true,"body":"When reading packed objects we know to tell apart missing objects and\ncorrupt objects by returning a positive error code in the former case,\nand a negative one in the latter case. We do that by distinguishing\nbetween errors returned by `find_pack_entry()`, which yields the offset\nof the object, and `packed_object_info()`, which reads the object\ncontents.\n\nBut even though we already distinguish those cases when reading packed\nobjects, the logic is broken in case a caller tries to read an object\nthat has been marked as corrupt. In that case, `find_pack_entry()` will\ntell us that the object in question does not exist, and consequently\nwe'll not flag the object as corrupt but as missing.\n\nFix this issue by bubbling up whether the object is corrupt and, if so,\nwhich packfile contains the corrupted object.\n\nNote that we don't yet need the information about the specific packfile,\nso we could've just as well made this a `bool *corrupted` pointer. But\nwe'll need information about the containing packfile in a subsequent\ncommit so that we can generate a proper error message telling the user\nwhich packfile contains the broken object.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/pack-objects.c | 2 +-\n midx.c | 10 +++++++---\n midx.h | 3 ++-\n odb/source-packed.c | 22 ++++++++++++++++------\n packfile.c | 10 +++++++---\n packfile.h | 3 ++-\n t/helper/test-read-midx.c | 2 +-\n 7 files changed, 36 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 1ec5b6f206..10c2471024 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -1786,7 +1786,7 @@ static int want_object_in_pack_mtime(const struct object_id *oid,\n \t\tstruct multi_pack_index *m = get_multi_pack_index(files->packed);\n \t\tstruct pack_entry e;\n \n-\t\tif (m && fill_midx_entry(m, oid, &e)) {\n+\t\tif (m && fill_midx_entry(m, oid, &e, NULL)) {\n \t\t\twant = want_object_in_pack_one(e.p, oid, exclude, found_pack, found_offset, found_mtime);\n \t\t\tif (want != -1)\n \t\t\t\treturn want;\ndiff --git a/midx.c b/midx.c\nindex 76c3f92cc3..37f082dbdd 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -591,7 +591,8 @@ uint32_t nth_midxed_pack_int_id(struct multi_pack_index *m, uint32_t pos)\n \n int fill_midx_entry(struct multi_pack_index *m,\n \t\t const struct object_id *oid,\n-\t\t struct pack_entry *e)\n+\t\t struct pack_entry *e,\n+\t\t struct packed_git **bad_pack)\n {\n \tuint32_t pos;\n \tuint32_t pack_int_id;\n@@ -618,8 +619,11 @@ int fill_midx_entry(struct multi_pack_index *m,\n \t\treturn 0;\n \n \tif (oidset_size(&p->bad_objects) &&\n-\t oidset_contains(&p->bad_objects, oid))\n+\t oidset_contains(&p->bad_objects, oid)) {\n+\t\tif (bad_pack && !*bad_pack)\n+\t\t\t*bad_pack = p;\n \t\treturn 0;\n+\t}\n \n \te->offset = nth_midxed_offset(m, pos);\n \te->p = p;\n@@ -1028,7 +1032,7 @@ int verify_midx_file(struct odb_source_packed *source, unsigned flags)\n \n \t\tnth_midxed_object_oid(&oid, m, pairs[i].pos);\n \n-\t\tif (!fill_midx_entry(m, &oid, &e)) {\n+\t\tif (!fill_midx_entry(m, &oid, &e, NULL)) {\n \t\t\tmidx_report(_(\"failed to load pack entry for oid[%d] = %s\"),\n \t\t\t\t pairs[i].pos, oid_to_hex(&oid));\n \t\t\tcontinue;\ndiff --git a/midx.h b/midx.h\nindex 939c18e588..1f2f2d5321 100644\n--- a/midx.h\n+++ b/midx.h\n@@ -117,7 +117,8 @@ uint32_t nth_midxed_pack_int_id(struct multi_pack_index *m, uint32_t pos);\n struct object_id *nth_midxed_object_oid(struct object_id *oid,\n \t\t\t\t\tstruct multi_pack_index *m,\n \t\t\t\t\tuint32_t n);\n-int fill_midx_entry(struct multi_pack_index *m, const struct object_id *oid, struct pack_entry *e);\n+int fill_midx_entry(struct multi_pack_index *m, const struct object_id *oid,\n+\t\t struct pack_entry *e, struct packed_git **bad_pack);\n int midx_contains_pack(struct multi_pack_index *m,\n \t\t const char *idx_or_pack_name);\n int midx_layer_contains_pack(struct multi_pack_index *m,\ndiff --git a/odb/source-packed.c b/odb/source-packed.c\nindex 0890704e76..16fa4f5769 100644\n--- a/odb/source-packed.c\n+++ b/odb/source-packed.c\n@@ -13,18 +13,19 @@\n \n static int find_pack_entry(struct odb_source_packed *store,\n \t\t\t const struct object_id *oid,\n-\t\t\t struct pack_entry *e)\n+\t\t\t struct pack_entry *e,\n+\t\t\t struct packed_git **bad_pack)\n {\n \tstruct packfile_list_entry *l;\n \n \todb_source_prepare(&store->base, 0);\n-\tif (store->midx && fill_midx_entry(store->midx, oid, e))\n+\tif (store->midx && fill_midx_entry(store->midx, oid, e, bad_pack))\n \t\treturn 1;\n \n \tfor (l = store->packs.head; l; l = l->next) {\n \t\tstruct packed_git *p = l->pack;\n \n-\t\tif (!p->multi_pack_index && packfile_fill_entry(p, oid, e)) {\n+\t\tif (!p->multi_pack_index && packfile_fill_entry(p, oid, e, bad_pack)) {\n \t\t\tif (!store->skip_mru_updates)\n \t\t\t\tpackfile_list_prepend(&store->packs, p);\n \t\t\treturn 1;\n@@ -40,6 +41,7 @@ static int odb_source_packed_read_object_info(struct odb_source *source,\n \t\t\t\t\t enum object_info_flags flags)\n {\n \tstruct odb_source_packed *packed = odb_source_packed_downcast(source);\n+\tstruct packed_git *bad_pack = NULL;\n \tstruct pack_entry e;\n \tint ret;\n \n@@ -51,8 +53,16 @@ static int odb_source_packed_read_object_info(struct odb_source *source,\n \tif (flags & OBJECT_INFO_SECOND_READ)\n \t\todb_source_prepare(source, ODB_PREPARE_FLUSH_CACHES);\n \n-\tif (!find_pack_entry(packed, oid, &e))\n+\tif (!find_pack_entry(packed, oid, &e, &bad_pack)) {\n+\t\t/*\n+\t\t * The lookup may have failed because the object is known to be\n+\t\t * corrupt in one of the packfiles. Report the object as\n+\t\t * corrupt instead of missing in that case.\n+\t\t */\n+\t\tif (bad_pack)\n+\t\t\treturn -1;\n \t\treturn 1;\n+\t}\n \n \t/*\n \t * We know that the caller doesn't actually need the\n@@ -77,7 +87,7 @@ static int odb_source_packed_read_object_stream(struct odb_read_stream **out,\n \tstruct odb_source_packed *packed = odb_source_packed_downcast(source);\n \tstruct pack_entry e;\n \n-\tif (!find_pack_entry(packed, oid, &e))\n+\tif (!find_pack_entry(packed, oid, &e, NULL))\n \t\treturn -1;\n \n \treturn packfile_read_object_stream(out, oid, e.p, e.offset);\n@@ -583,7 +593,7 @@ static int odb_source_packed_freshen_object(struct odb_source *source,\n \t\ttimesp = ×\n \t}\n \n-\tif (!find_pack_entry(packed, oid, &e))\n+\tif (!find_pack_entry(packed, oid, &e, NULL))\n \t\treturn 0;\n \tif (e.p->is_cruft)\n \t\treturn 0;\ndiff --git a/packfile.c b/packfile.c\nindex 0eee45055f..34e2f9bb8b 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1859,13 +1859,17 @@ int is_pack_valid(struct packed_git *p)\n \n int packfile_fill_entry(struct packed_git *p,\n \t\t\tconst struct object_id *oid,\n-\t\t\tstruct pack_entry *e)\n+\t\t\tstruct pack_entry *e,\n+\t\t\tstruct packed_git **bad_pack)\n {\n \toff_t offset;\n \n \tif (oidset_size(&p->bad_objects) &&\n-\t oidset_contains(&p->bad_objects, oid))\n+\t oidset_contains(&p->bad_objects, oid)) {\n+\t\tif (bad_pack && !*bad_pack)\n+\t\t\t*bad_pack = p;\n \t\treturn 0;\n+\t}\n \n \toffset = find_pack_entry_one(oid, p);\n \tif (!offset)\n@@ -1962,7 +1966,7 @@ int has_object_kept_pack(struct repository *r, const struct object_id *oid,\n \n \t\tfor (; *cache; cache++) {\n \t\t\tstruct packed_git *p = *cache;\n-\t\t\tif (packfile_fill_entry(p, oid, &e))\n+\t\t\tif (packfile_fill_entry(p, oid, &e, NULL))\n \t\t\t\treturn 1;\n \t\t}\n \t}\ndiff --git a/packfile.h b/packfile.h\nindex e1f77152b5..3229a6ed47 100644\n--- a/packfile.h\n+++ b/packfile.h\n@@ -294,7 +294,8 @@ off_t find_pack_entry_one(const struct object_id *oid, struct packed_git *);\n \n int packfile_fill_entry(struct packed_git *p,\n \t\t\tconst struct object_id *oid,\n-\t\t\tstruct pack_entry *e);\n+\t\t\tstruct pack_entry *e,\n+\t\t\tstruct packed_git **bad_pack);\n \n int is_pack_valid(struct packed_git *);\n void *unpack_entry(struct repository *r, struct packed_git *, off_t,\ndiff --git a/t/helper/test-read-midx.c b/t/helper/test-read-midx.c\nindex fb16ec0176..27a05da957 100644\n--- a/t/helper/test-read-midx.c\n+++ b/t/helper/test-read-midx.c\n@@ -82,7 +82,7 @@ static int read_midx_file(const char *object_dir, const char *checksum,\n \t\tfor (i = 0; i < m->num_objects; i++) {\n \t\t\tnth_midxed_object_oid(&oid, m,\n \t\t\t\t\t i + m->num_objects_in_base);\n-\t\t\tfill_midx_entry(m, &oid, &e);\n+\t\t\tfill_midx_entry(m, &oid, &e, NULL);\n \n \t\t\tprintf(\"%s %\"PRIu64\"\\t%s\\n\",\n \t\t\t oid_to_hex(&oid), e.offset, e.p->pack_name);\n\n-- \n2.55.0.822.g20453c30eb.dirty\n\n"},{"id":"550805","messageId":"20260819-pks-odb-generic-corrupt-objects-v2-2-a984e3a0ad6f@pks.im","threadId":"66194","inReplyTo":"20260819-pks-odb-generic-corrupt-objects-v2-0-a984e3a0ad6f@pks.im","subject":"[PATCH v2 2/5] odb/source: introduce error status when reading objects","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-19T12:17:20Z","receivedAt":"2026-08-19T12:17:54Z","isPatch":true,"body":"The `read_object_info()` callback of `struct odb_source` is documented\nto return a negative error code in case reading the object has failed,\nand zero otherwise. This is overly broad though, as there are two very\ndifferent kinds of failures:\n\n - The object may not exist in the source at all.\n\n - The object exists, but reading it has failed, for example because\n its on-disk state is corrupt.\n\nThis distinction matters to callers: when an object is corrupt in one\nsource we may still find a good copy of it in another source, so we may\nstill be able to proceed with a given operation.\n\nThe \"packed\" source already distinguishes these cases by returning a\npositive value for missing objects and a negative value in case reading\nthe object has failed. But it is the only such source that distinguishes\nthose cases, and the returned value is translated into a negative error\ncode by the \"files\" backend anyway.\n\nIntroduce a new error status that is specific to reading objects and\nadapt the infrastructure to return it. For now, we only discern\nsuccessful reads from generic failures, which mostly matches the status\nquo. In subsequent commits though we're about to add an error that\nexplicitly tells the caller that an object does not exist.\n\nNote that we keep the \"packed\" backend as-is with its positive return\ncode for missing objects. This will be fixed in the next commit.\n\nSigned-off-by: Patrick Steinhardt \n---\n odb.c | 16 ++++++++--------\n odb.h | 15 +++++++++++----\n odb/source-files.c | 8 ++++----\n odb/source-inmemory.c | 8 ++++----\n odb/source-loose.c | 8 ++++----\n odb/source-packed.c | 8 ++++----\n odb/source.h | 22 +++++++++++-----------\n 7 files changed, 46 insertions(+), 39 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex caf1d0f542..1b37b26376 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -547,9 +547,9 @@ static int register_all_submodule_sources(struct object_database *odb)\n \treturn ret;\n }\n \n-static int do_oid_object_info_extended(struct object_database *odb,\n-\t\t\t\t const struct object_id *oid,\n-\t\t\t\t struct object_info *oi, unsigned flags)\n+static enum odb_read_status do_oid_object_info_extended(struct object_database *odb,\n+\t\t\t\t\t\t\tconst struct object_id *oid,\n+\t\t\t\t\t\t\tstruct object_info *oi, unsigned flags)\n {\n \tconst struct object_id *real = oid;\n \tint already_retried = 0;\n@@ -696,12 +696,12 @@ static int oid_object_info_convert(struct repository *r,\n \treturn ret;\n }\n \n-int odb_read_object_info_extended(struct object_database *odb,\n-\t\t\t\t const struct object_id *oid,\n-\t\t\t\t struct object_info *oi,\n-\t\t\t\t enum object_info_flags flags)\n+enum odb_read_status odb_read_object_info_extended(struct object_database *odb,\n+\t\t\t\t\t\t const struct object_id *oid,\n+\t\t\t\t\t\t struct object_info *oi,\n+\t\t\t\t\t\t enum object_info_flags flags)\n {\n-\tint ret;\n+\tenum odb_read_status ret;\n \n \tif (oid->algo && (hash_algo_by_ptr(odb->repo->hash_algo) != oid->algo))\n \t\treturn oid_object_info_convert(odb->repo, oid, oi, flags);\ndiff --git a/odb.h b/odb.h\nindex fca67e8253..43cbcc3aba 100644\n--- a/odb.h\n+++ b/odb.h\n@@ -435,14 +435,21 @@ enum object_info_flags {\n \tOBJECT_INFO_FOR_PREFETCH = (OBJECT_INFO_SKIP_FETCH_OBJECT | OBJECT_INFO_QUICK),\n };\n \n+enum odb_read_status {\n+\t/* The read was successful. */\n+\tODB_READ_OK = 0,\n+\t/* The read resulted in a generic error. */\n+\tODB_READ_ERROR = -1,\n+};\n+\n /*\n * Read object info from the object database and populate the `object_info`\n * structure. Returns 0 on success, a negative error code otherwise.\n */\n-int odb_read_object_info_extended(struct object_database *odb,\n-\t\t\t\t const struct object_id *oid,\n-\t\t\t\t struct object_info *oi,\n-\t\t\t\t enum object_info_flags flags);\n+enum odb_read_status odb_read_object_info_extended(struct object_database *odb,\n+\t\t\t\t\t\t const struct object_id *oid,\n+\t\t\t\t\t\t struct object_info *oi,\n+\t\t\t\t\t\t enum object_info_flags flags);\n \n /*\n * Read a subset of object info for the given object ID. Returns an `enum\ndiff --git a/odb/source-files.c b/odb/source-files.c\nindex 5a68af7d84..a28aa5042d 100644\n--- a/odb/source-files.c\n+++ b/odb/source-files.c\n@@ -59,10 +59,10 @@ static void odb_source_files_prepare(struct odb_source *source,\n \todb_source_prepare(&files->packed->base, flags);\n }\n \n-static int odb_source_files_read_object_info(struct odb_source *source,\n-\t\t\t\t\t const struct object_id *oid,\n-\t\t\t\t\t struct object_info *oi,\n-\t\t\t\t\t enum object_info_flags flags)\n+static enum odb_read_status odb_source_files_read_object_info(struct odb_source *source,\n+\t\t\t\t\t\t\t const struct object_id *oid,\n+\t\t\t\t\t\t\t struct object_info *oi,\n+\t\t\t\t\t\t\t enum object_info_flags flags)\n {\n \tstruct odb_source_files *files = odb_source_files_downcast(source);\n \ndiff --git a/odb/source-inmemory.c b/odb/source-inmemory.c\nindex 3e71611b8e..53d2e3a852 100644\n--- a/odb/source-inmemory.c\n+++ b/odb/source-inmemory.c\n@@ -56,10 +56,10 @@ static void populate_object_info(struct odb_source_inmemory *source,\n \t\toi->source_infop->source = &source->base;\n }\n \n-static int odb_source_inmemory_read_object_info(struct odb_source *source,\n-\t\t\t\t\t\tconst struct object_id *oid,\n-\t\t\t\t\t\tstruct object_info *oi,\n-\t\t\t\t\t\tenum object_info_flags flags UNUSED)\n+static enum odb_read_status odb_source_inmemory_read_object_info(struct odb_source *source,\n+\t\t\t\t\t\t\t\t const struct object_id *oid,\n+\t\t\t\t\t\t\t\t struct object_info *oi,\n+\t\t\t\t\t\t\t\t enum object_info_flags flags UNUSED)\n {\n \tstruct odb_source_inmemory *inmemory = odb_source_inmemory_downcast(source);\n \tconst struct inmemory_object *object;\ndiff --git a/odb/source-loose.c b/odb/source-loose.c\nindex ef0e919277..ad8662842d 100644\n--- a/odb/source-loose.c\n+++ b/odb/source-loose.c\n@@ -206,10 +206,10 @@ static int read_object_info_from_path(struct odb_source_loose *loose,\n \treturn ret;\n }\n \n-static int odb_source_loose_read_object_info(struct odb_source *source,\n-\t\t\t\t\t const struct object_id *oid,\n-\t\t\t\t\t struct object_info *oi,\n-\t\t\t\t\t enum object_info_flags flags)\n+static enum odb_read_status odb_source_loose_read_object_info(struct odb_source *source,\n+\t\t\t\t\t\t\t const struct object_id *oid,\n+\t\t\t\t\t\t\t struct object_info *oi,\n+\t\t\t\t\t\t\t enum object_info_flags flags)\n {\n \tstruct odb_source_loose *loose = odb_source_loose_downcast(source);\n \tstatic struct strbuf buf = STRBUF_INIT;\ndiff --git a/odb/source-packed.c b/odb/source-packed.c\nindex 16fa4f5769..dce68a57f7 100644\n--- a/odb/source-packed.c\n+++ b/odb/source-packed.c\n@@ -35,10 +35,10 @@ static int find_pack_entry(struct odb_source_packed *store,\n \treturn 0;\n }\n \n-static int odb_source_packed_read_object_info(struct odb_source *source,\n-\t\t\t\t\t const struct object_id *oid,\n-\t\t\t\t\t struct object_info *oi,\n-\t\t\t\t\t enum object_info_flags flags)\n+static enum odb_read_status odb_source_packed_read_object_info(struct odb_source *source,\n+\t\t\t\t\t\t\t const struct object_id *oid,\n+\t\t\t\t\t\t\t struct object_info *oi,\n+\t\t\t\t\t\t\t enum object_info_flags flags)\n {\n \tstruct odb_source_packed *packed = odb_source_packed_downcast(source);\n \tstruct packed_git *bad_pack = NULL;\ndiff --git a/odb/source.h b/odb/source.h\nindex d69f8e2d1c..7b8ff3d19d 100644\n--- a/odb/source.h\n+++ b/odb/source.h\n@@ -110,13 +110,13 @@ struct odb_source {\n \t * second read in case they know that the first read would have\n \t * already surfaced the object without reloading any on-disk state.\n \t *\n-\t * The callback is expected to return a negative error code in case\n-\t * reading the object has failed, 0 otherwise.\n+\t * The callback is expected to return an `enum odb_read_status`. Please\n+\t * refer to the individual values that can be returned.\n \t */\n-\tint (*read_object_info)(struct odb_source *source,\n-\t\t\t\tconst struct object_id *oid,\n-\t\t\t\tstruct object_info *oi,\n-\t\t\t\tenum object_info_flags flags);\n+\tenum odb_read_status (*read_object_info)(struct odb_source *source,\n+\t\t\t\t\t\t const struct object_id *oid,\n+\t\t\t\t\t\t struct object_info *oi,\n+\t\t\t\t\t\t enum object_info_flags flags);\n \n \t/*\n \t * This callback is expected to create a new read stream that can be\n@@ -340,12 +340,12 @@ static inline void odb_source_prepare(struct odb_source *source,\n \n /*\n * Read an object from the object database source identified by its object ID.\n- * Returns 0 on success, a negative error code otherwise.\n+ * Please refer to `enum odb_read_status` for the individual error codes.\n */\n-static inline int odb_source_read_object_info(struct odb_source *source,\n-\t\t\t\t\t const struct object_id *oid,\n-\t\t\t\t\t struct object_info *oi,\n-\t\t\t\t\t enum object_info_flags flags)\n+static inline enum odb_read_status odb_source_read_object_info(struct odb_source *source,\n+\t\t\t\t\t\t\t const struct object_id *oid,\n+\t\t\t\t\t\t\t struct object_info *oi,\n+\t\t\t\t\t\t\t enum object_info_flags flags)\n {\n \treturn source->read_object_info(source, oid, oi, flags);\n }\n\n-- \n2.55.0.822.g20453c30eb.dirty\n\n"},{"id":"550806","messageId":"20260819-pks-odb-generic-corrupt-objects-v2-3-a984e3a0ad6f@pks.im","threadId":"66194","inReplyTo":"20260819-pks-odb-generic-corrupt-objects-v2-0-a984e3a0ad6f@pks.im","subject":"[PATCH v2 3/5] odb/source: let callers discern missing and corrupt objects","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-19T12:17:21Z","receivedAt":"2026-08-19T12:17:57Z","isPatch":true,"body":"As explained in the preceding commits, reading objects can either fail\nbecause the object truly does not exist or because it exists, but its\ndata is corrupt. Some callers do care about this distinction, but there\nis no way to tell these two cases apart right now.\n\nIntroduce a new `ODB_READ_NOT_FOUND` value that ought to be returned by\nthe backends in case the object truly does not exist and adapt backends\nto use it.\n\nNote that we don't yet return this error from `odb_read_object_info()`\nitself. This will be fixed in a subsequent commit.\n\nSigned-off-by: Patrick Steinhardt \n---\n odb.h | 2 ++\n odb/source-files.c | 20 +++++++++++++++++---\n odb/source-inmemory.c | 2 +-\n odb/source-loose.c | 31 +++++++++++++++++++------------\n odb/source-packed.c | 2 +-\n t/unit-tests/u-odb-inmemory.c | 3 ++-\n 6 files changed, 42 insertions(+), 18 deletions(-)\n\ndiff --git a/odb.h b/odb.h\nindex 43cbcc3aba..1264d4ce7d 100644\n--- a/odb.h\n+++ b/odb.h\n@@ -440,6 +440,8 @@ enum odb_read_status {\n \tODB_READ_OK = 0,\n \t/* The read resulted in a generic error. */\n \tODB_READ_ERROR = -1,\n+\t/* The object could not be found. */\n+\tODB_READ_NOT_FOUND = -2,\n };\n \n /*\ndiff --git a/odb/source-files.c b/odb/source-files.c\nindex a28aa5042d..e88fd1d399 100644\n--- a/odb/source-files.c\n+++ b/odb/source-files.c\n@@ -65,12 +65,26 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source\n \t\t\t\t\t\t\t enum object_info_flags flags)\n {\n \tstruct odb_source_files *files = odb_source_files_downcast(source);\n+\tenum odb_read_status ret_packed, ret_loose;\n \n-\tif (!odb_source_read_object_info(&files->packed->base, oid, oi, flags) ||\n-\t !odb_source_read_object_info(&files->loose->base, oid, oi, flags))\n+\tret_packed = odb_source_read_object_info(&files->packed->base, oid, oi, flags);\n+\tif (!ret_packed)\n \t\treturn 0;\n \n-\treturn -1;\n+\tret_loose = odb_source_read_object_info(&files->loose->base, oid, oi, flags);\n+\tif (!ret_loose)\n+\t\treturn 0;\n+\n+\t/*\n+\t * Reading the packed object may have failed even though the object\n+\t * exists, for example because it is corrupt. Report this failure to\n+\t * the caller in case neither of the sources was able to read the\n+\t * object, and prefer the error of the packed source in case both\n+\t * reads have failed.\n+\t */\n+\tif (ret_packed != ODB_READ_NOT_FOUND)\n+\t\treturn ret_packed;\n+\treturn ret_loose;\n }\n \n static int odb_source_files_read_object_stream(struct odb_read_stream **out,\ndiff --git a/odb/source-inmemory.c b/odb/source-inmemory.c\nindex 53d2e3a852..3f3bd12de3 100644\n--- a/odb/source-inmemory.c\n+++ b/odb/source-inmemory.c\n@@ -66,7 +66,7 @@ static enum odb_read_status odb_source_inmemory_read_object_info(struct odb_sour\n \n \tobject = find_cached_object(inmemory, oid);\n \tif (!object)\n-\t\treturn -1;\n+\t\treturn ODB_READ_NOT_FOUND;\n \n \tpopulate_object_info(inmemory, oi, object);\n \treturn 0;\ndiff --git a/odb/source-loose.c b/odb/source-loose.c\nindex ad8662842d..3c942a1069 100644\n--- a/odb/source-loose.c\n+++ b/odb/source-loose.c\n@@ -91,11 +91,16 @@ static int read_object_info_from_path(struct odb_source_loose *loose,\n \t\tstruct stat st;\n \n \t\tif ((!oi || (!oi->disk_sizep && !oi->mtimep)) && (flags & OBJECT_INFO_QUICK)) {\n-\t\t\tret = quick_has_loose(loose, oid) ? 0 : -1;\n+\t\t\tret = quick_has_loose(loose, oid) ? 0 : ODB_READ_NOT_FOUND;\n \t\t\tgoto out;\n \t\t}\n \n \t\tif (lstat(path, &st) < 0) {\n+\t\t\tif (errno == ENOENT) {\n+\t\t\t\tret = ODB_READ_NOT_FOUND;\n+\t\t\t\tgoto out;\n+\t\t\t}\n+\n \t\t\tret = -1;\n \t\t\tgoto out;\n \t\t}\n@@ -113,9 +118,12 @@ static int read_object_info_from_path(struct odb_source_loose *loose,\n \n \tfd = git_open(path);\n \tif (fd < 0) {\n-\t\tif (errno != ENOENT)\n-\t\t\terror_errno(_(\"unable to open loose object %s\"), oid_to_hex(oid));\n-\t\tret = -1;\n+\t\tif (errno == ENOENT) {\n+\t\t\tret = ODB_READ_NOT_FOUND;\n+\t\t\tgoto out;\n+\t\t}\n+\n+\t\tret = error_errno(_(\"unable to open loose object %s\"), oid_to_hex(oid));\n \t\tgoto out;\n \t}\n \n@@ -155,7 +163,7 @@ static int read_object_info_from_path(struct odb_source_loose *loose,\n \n \t\tif (parse_loose_header(hdr, oi) < 0) {\n \t\t\tret = error(_(\"unable to parse %s header\"), oid_to_hex(oid));\n-\t\t\tgoto corrupt;\n+\t\t\tgoto out;\n \t\t}\n \n \t\tif (*oi->typep < 0)\n@@ -165,7 +173,7 @@ static int read_object_info_from_path(struct odb_source_loose *loose,\n \t\t\t*oi->contentp = unpack_loose_rest(&stream, hdr, *oi->sizep, oid);\n \t\t\tif (!*oi->contentp) {\n \t\t\t\tret = -1;\n-\t\t\t\tgoto corrupt;\n+\t\t\t\tgoto out;\n \t\t\t}\n \t\t}\n \n@@ -173,21 +181,20 @@ static int read_object_info_from_path(struct odb_source_loose *loose,\n \tcase ULHR_BAD:\n \t\tret = error(_(\"unable to unpack %s header\"),\n \t\t\t oid_to_hex(oid));\n-\t\tgoto corrupt;\n+\t\tgoto out;\n \tcase ULHR_TOO_LONG:\n \t\tret = error(_(\"header for %s too long, exceeds %d bytes\"),\n \t\t\t oid_to_hex(oid), MAX_HEADER_LEN);\n-\t\tgoto corrupt;\n+\t\tgoto out;\n \t}\n \n \tret = 0;\n \n-corrupt:\n-\tif (ret && (flags & OBJECT_INFO_DIE_IF_CORRUPT))\n+out:\n+\tif (ret && ret != ODB_READ_NOT_FOUND && (flags & OBJECT_INFO_DIE_IF_CORRUPT))\n \t\tdie(_(\"loose object %s (stored in %s) is corrupt\"),\n \t\t oid_to_hex(oid), path);\n \n-out:\n \tif (stream_to_end)\n \t\tgit_inflate_end(stream_to_end);\n \tif (map)\n@@ -221,7 +228,7 @@ static enum odb_read_status odb_source_loose_read_object_info(struct odb_source\n \t * second time.\n \t */\n \tif (flags & OBJECT_INFO_SECOND_READ)\n-\t\treturn -1;\n+\t\treturn ODB_READ_NOT_FOUND;\n \n \todb_loose_path(loose, &buf, oid);\n \treturn read_object_info_from_path(loose, buf.buf, oid, oi, flags);\ndiff --git a/odb/source-packed.c b/odb/source-packed.c\nindex dce68a57f7..9b19405380 100644\n--- a/odb/source-packed.c\n+++ b/odb/source-packed.c\n@@ -61,7 +61,7 @@ static enum odb_read_status odb_source_packed_read_object_info(struct odb_source\n \t\t */\n \t\tif (bad_pack)\n \t\t\treturn -1;\n-\t\treturn 1;\n+\t\treturn ODB_READ_NOT_FOUND;\n \t}\n \n \t/*\ndiff --git a/t/unit-tests/u-odb-inmemory.c b/t/unit-tests/u-odb-inmemory.c\nindex ddf2db5c81..3e5068080c 100644\n--- a/t/unit-tests/u-odb-inmemory.c\n+++ b/t/unit-tests/u-odb-inmemory.c\n@@ -72,7 +72,8 @@ void test_odb_inmemory__read_missing_object(void)\n \tconst char *end;\n \n \tcl_must_pass(parse_oid_hex_algop(RANDOM_OID, &oid, &end, repo.hash_algo));\n-\tcl_must_fail(odb_source_read_object_info(&source->base, &oid, NULL, 0));\n+\tcl_assert_equal_i(odb_source_read_object_info(&source->base, &oid, NULL, 0),\n+\t\t\t ODB_READ_NOT_FOUND);\n \n \todb_source_free(&source->base);\n }\n\n-- \n2.55.0.822.g20453c30eb.dirty\n\n"},{"id":"550807","messageId":"20260819-pks-odb-generic-corrupt-objects-v2-4-a984e3a0ad6f@pks.im","threadId":"66194","inReplyTo":"20260819-pks-odb-generic-corrupt-objects-v2-0-a984e3a0ad6f@pks.im","subject":"[PATCH v2 4/5] odb/source: allow `read_object_info()` to bubble up error messages","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-19T12:17:22Z","receivedAt":"2026-08-19T12:18:00Z","isPatch":true,"body":"When reading an object fails even though it exists, the sources know\nbest what exactly went wrong and where the corrupt object is located.\nThis information is lost though when bubbling up the error to the object\ndatabase layer, which forces that layer to reconstruct it after the\nfact. This is exactly what `do_oid_object_info_extended()` does via\n`has_packed_and_bad()`, but that function only really knows to handle\nthe \"files\" backend by reaching into its internals.\n\nIntroduce a new `errmsg` parameter for the `read_object_info()` callback\nthat sources are expected to populate with a human-readable message in\ncase reading the object has failed. Adapt the packed and loose sources\nto populate the buffer with the messages that we ultimately want to\nsurface to the user.\n\nFor now, all callers are adapted to pass a `NULL` pointer. We will add a\nuser of this new infrastructure in a subsequent commit.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/pack-objects.c | 6 +++---\n odb.c | 7 ++++---\n odb/source-files.c | 9 ++++++---\n odb/source-inmemory.c | 3 ++-\n odb/source-loose.c | 23 +++++++++++++++--------\n odb/source-packed.c | 34 ++++++++++++++++++++++++++--------\n odb/source.h | 18 ++++++++++++++----\n packfile.c | 2 +-\n t/unit-tests/u-odb-inmemory.c | 4 ++--\n 9 files changed, 73 insertions(+), 33 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 10c2471024..399acd0f22 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -1759,7 +1759,7 @@ static int want_object_in_pack_mtime(const struct object_id *oid,\n \t\tstruct odb_source *source = the_repository->objects->sources->next;\n \t\tfor (; source; source = source->next) {\n \t\t\tstruct odb_source_files *files = odb_source_files_downcast(source);\n-\t\t\tif (!odb_source_read_object_info(&files->loose->base, oid, NULL, 0))\n+\t\t\tif (!odb_source_read_object_info(&files->loose->base, oid, NULL, 0, NULL))\n \t\t\t\treturn 0;\n \t\t}\n \t}\n@@ -4171,7 +4171,7 @@ static void add_cruft_object_entry(const struct object_id *oid, enum object_type\n \n \t\t\tfor (; !found && source; source = source->next) {\n \t\t\t\tstruct odb_source_files *files = odb_source_files_downcast(source);\n-\t\t\t\tif (!odb_source_read_object_info(&files->loose->base, oid, NULL, 0))\n+\t\t\t\tif (!odb_source_read_object_info(&files->loose->base, oid, NULL, 0, NULL))\n \t\t\t\t\tfound = 1;\n \t\t\t}\n \n@@ -4637,7 +4637,7 @@ static int force_object_loose(struct odb_source *source,\n \n \tfor (struct odb_source *s = source->odb->sources; s; s = s->next) {\n \t\tstruct odb_source_files *files = odb_source_files_downcast(s);\n-\t\tif (!odb_source_read_object_info(&files->loose->base, oid, NULL, 0))\n+\t\tif (!odb_source_read_object_info(&files->loose->base, oid, NULL, 0, NULL))\n \t\t\treturn 0;\n \t}\n \ndiff --git a/odb.c b/odb.c\nindex 1b37b26376..83a53f7f6b 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -560,7 +560,7 @@ static enum odb_read_status do_oid_object_info_extended(struct object_database *\n \tif (is_null_oid(real))\n \t\treturn -1;\n \n-\tif (!odb_source_read_object_info(odb->inmemory_objects, oid, oi, flags))\n+\tif (!odb_source_read_object_info(odb->inmemory_objects, oid, oi, flags, NULL))\n \t\treturn 0;\n \n \todb_prepare_alternates(odb);\n@@ -569,7 +569,7 @@ static enum odb_read_status do_oid_object_info_extended(struct object_database *\n \t\tstruct odb_source *source;\n \n \t\tfor (source = odb->sources; source; source = source->next)\n-\t\t\tif (!odb_source_read_object_info(source, real, oi, flags))\n+\t\t\tif (!odb_source_read_object_info(source, real, oi, flags, NULL))\n \t\t\t\treturn 0;\n \n \t\t/*\n@@ -580,7 +580,8 @@ static enum odb_read_status do_oid_object_info_extended(struct object_database *\n \t\tif (!(flags & OBJECT_INFO_QUICK)) {\n \t\t\tfor (source = odb->sources; source; source = source->next)\n \t\t\t\tif (!odb_source_read_object_info(source, real, oi,\n-\t\t\t\t\t\t\t\t flags | OBJECT_INFO_SECOND_READ))\n+\t\t\t\t\t\t\t\t flags | OBJECT_INFO_SECOND_READ,\n+\t\t\t\t\t\t\t\t NULL))\n \t\t\t\t\treturn 0;\n \t\t}\n \ndiff --git a/odb/source-files.c b/odb/source-files.c\nindex e88fd1d399..aafba358e4 100644\n--- a/odb/source-files.c\n+++ b/odb/source-files.c\n@@ -62,16 +62,19 @@ static void odb_source_files_prepare(struct odb_source *source,\n static enum odb_read_status odb_source_files_read_object_info(struct odb_source *source,\n \t\t\t\t\t\t\t const struct object_id *oid,\n \t\t\t\t\t\t\t struct object_info *oi,\n-\t\t\t\t\t\t\t enum object_info_flags flags)\n+\t\t\t\t\t\t\t enum object_info_flags flags,\n+\t\t\t\t\t\t\t struct strbuf *errmsg)\n {\n \tstruct odb_source_files *files = odb_source_files_downcast(source);\n \tenum odb_read_status ret_packed, ret_loose;\n \n-\tret_packed = odb_source_read_object_info(&files->packed->base, oid, oi, flags);\n+\tret_packed = odb_source_read_object_info(&files->packed->base, oid, oi,\n+\t\t\t\t\t\t flags, errmsg);\n \tif (!ret_packed)\n \t\treturn 0;\n \n-\tret_loose = odb_source_read_object_info(&files->loose->base, oid, oi, flags);\n+\tret_loose = odb_source_read_object_info(&files->loose->base, oid, oi, flags,\n+\t\t\t\t\t\tret_packed == ODB_READ_NOT_FOUND ? errmsg : NULL);\n \tif (!ret_loose)\n \t\treturn 0;\n \ndiff --git a/odb/source-inmemory.c b/odb/source-inmemory.c\nindex 3f3bd12de3..12f91e594a 100644\n--- a/odb/source-inmemory.c\n+++ b/odb/source-inmemory.c\n@@ -59,7 +59,8 @@ static void populate_object_info(struct odb_source_inmemory *source,\n static enum odb_read_status odb_source_inmemory_read_object_info(struct odb_source *source,\n \t\t\t\t\t\t\t\t const struct object_id *oid,\n \t\t\t\t\t\t\t\t struct object_info *oi,\n-\t\t\t\t\t\t\t\t enum object_info_flags flags UNUSED)\n+\t\t\t\t\t\t\t\t enum object_info_flags flags UNUSED,\n+\t\t\t\t\t\t\t\t struct strbuf *errmsg UNUSED)\n {\n \tstruct odb_source_inmemory *inmemory = odb_source_inmemory_downcast(source);\n \tconst struct inmemory_object *object;\ndiff --git a/odb/source-loose.c b/odb/source-loose.c\nindex 3c942a1069..b57ee2701a 100644\n--- a/odb/source-loose.c\n+++ b/odb/source-loose.c\n@@ -67,7 +67,8 @@ static int read_object_info_from_path(struct odb_source_loose *loose,\n \t\t\t\t const char *path,\n \t\t\t\t const struct object_id *oid,\n \t\t\t\t struct object_info *oi,\n-\t\t\t\t enum object_info_flags flags)\n+\t\t\t\t enum object_info_flags flags,\n+\t\t\t\t struct strbuf *errmsg)\n {\n \tint ret;\n \tint fd;\n@@ -191,9 +192,14 @@ static int read_object_info_from_path(struct odb_source_loose *loose,\n \tret = 0;\n \n out:\n-\tif (ret && ret != ODB_READ_NOT_FOUND && (flags & OBJECT_INFO_DIE_IF_CORRUPT))\n-\t\tdie(_(\"loose object %s (stored in %s) is corrupt\"),\n-\t\t oid_to_hex(oid), path);\n+\tif (ret && ret != ODB_READ_NOT_FOUND) {\n+\t\tif ((flags & OBJECT_INFO_DIE_IF_CORRUPT))\n+\t\t\tdie(_(\"loose object %s (stored in %s) is corrupt\"),\n+\t\t\t oid_to_hex(oid), path);\n+\t\tif (errmsg)\n+\t\t\tstrbuf_addf(errmsg, _(\"loose object %s (stored in %s) is corrupt\"),\n+\t\t\t\t oid_to_hex(oid), path);\n+\t}\n \n \tif (stream_to_end)\n \t\tgit_inflate_end(stream_to_end);\n@@ -216,7 +222,8 @@ static int read_object_info_from_path(struct odb_source_loose *loose,\n static enum odb_read_status odb_source_loose_read_object_info(struct odb_source *source,\n \t\t\t\t\t\t\t const struct object_id *oid,\n \t\t\t\t\t\t\t struct object_info *oi,\n-\t\t\t\t\t\t\t enum object_info_flags flags)\n+\t\t\t\t\t\t\t enum object_info_flags flags,\n+\t\t\t\t\t\t\t struct strbuf *errmsg)\n {\n \tstruct odb_source_loose *loose = odb_source_loose_downcast(source);\n \tstatic struct strbuf buf = STRBUF_INIT;\n@@ -231,7 +238,7 @@ static enum odb_read_status odb_source_loose_read_object_info(struct odb_source\n \t\treturn ODB_READ_NOT_FOUND;\n \n \todb_loose_path(loose, &buf, oid);\n-\treturn read_object_info_from_path(loose, buf.buf, oid, oi, flags);\n+\treturn read_object_info_from_path(loose, buf.buf, oid, oi, flags, errmsg);\n }\n \n /*\n@@ -428,7 +435,7 @@ static int for_each_object_wrapper_cb(const struct object_id *oid,\n \tif (data->request) {\n \t\tstruct object_info oi = *data->request;\n \n-\t\tif (read_object_info_from_path(data->loose, path, oid, &oi, 0) < 0)\n+\t\tif (read_object_info_from_path(data->loose, path, oid, &oi, 0, NULL) < 0)\n \t\t\treturn -1;\n \n \t\treturn data->cb(oid, &oi, data->cb_data);\n@@ -446,7 +453,7 @@ static int for_each_prefixed_object_wrapper_cb(const struct object_id *oid,\n \t\tstruct object_info oi = *data->request;\n \n \t\tif (odb_source_read_object_info(&data->loose->base,\n-\t\t\t\t\t\toid, &oi, 0) < 0)\n+\t\t\t\t\t\toid, &oi, 0, NULL) < 0)\n \t\t\treturn -1;\n \n \t\treturn data->cb(oid, &oi, data->cb_data);\ndiff --git a/odb/source-packed.c b/odb/source-packed.c\nindex 9b19405380..1a12a605db 100644\n--- a/odb/source-packed.c\n+++ b/odb/source-packed.c\n@@ -2,7 +2,9 @@\n #include \"abspath.h\"\n #include \"chdir-notify.h\"\n #include \"dir.h\"\n+#include \"gettext.h\"\n #include \"git-zlib.h\"\n+#include \"hex.h\"\n #include \"list-objects-filter-options.h\"\n #include \"mergesort.h\"\n #include \"midx.h\"\n@@ -10,6 +12,7 @@\n #include \"odb/streaming.h\"\n #include \"packfile.h\"\n #include \"pack-bitmap.h\"\n+#include \"strbuf.h\"\n \n static int find_pack_entry(struct odb_source_packed *store,\n \t\t\t const struct object_id *oid,\n@@ -38,7 +41,8 @@ static int find_pack_entry(struct odb_source_packed *store,\n static enum odb_read_status odb_source_packed_read_object_info(struct odb_source *source,\n \t\t\t\t\t\t\t const struct object_id *oid,\n \t\t\t\t\t\t\t struct object_info *oi,\n-\t\t\t\t\t\t\t enum object_info_flags flags)\n+\t\t\t\t\t\t\t enum object_info_flags flags,\n+\t\t\t\t\t\t\t struct strbuf *errmsg)\n {\n \tstruct odb_source_packed *packed = odb_source_packed_downcast(source);\n \tstruct packed_git *bad_pack = NULL;\n@@ -59,25 +63,39 @@ static enum odb_read_status odb_source_packed_read_object_info(struct odb_source\n \t\t * corrupt in one of the packfiles. Report the object as\n \t\t * corrupt instead of missing in that case.\n \t\t */\n-\t\tif (bad_pack)\n-\t\t\treturn -1;\n-\t\treturn ODB_READ_NOT_FOUND;\n+\t\tif (bad_pack) {\n+\t\t\tret = -1;\n+\t\t\tgoto out;\n+\t\t}\n+\n+\t\tret = ODB_READ_NOT_FOUND;\n+\t\tgoto out;\n \t}\n \n \t/*\n \t * We know that the caller doesn't actually need the\n \t * information below, so return early.\n \t */\n-\tif (!oi)\n-\t\treturn 0;\n+\tif (!oi) {\n+\t\tret = 0;\n+\t\tgoto out;\n+\t}\n \n \tret = packed_object_info(packed, e.p, e.offset, oi);\n \tif (ret < 0) {\n+\t\tbad_pack = e.p;\n \t\tmark_bad_packed_object(e.p, oid);\n-\t\treturn -1;\n+\t\tgoto out;\n \t}\n \n-\treturn 0;\n+\tret = 0;\n+\n+out:\n+\tif (ret < 0 && bad_pack && errmsg)\n+\t\tstrbuf_addf(errmsg, _(\"packed object %s (stored in %s) is corrupt\"),\n+\t\t\t oid_to_hex(oid), bad_pack->pack_name);\n+\n+\treturn ret;\n }\n \n static int odb_source_packed_read_object_stream(struct odb_read_stream **out,\ndiff --git a/odb/source.h b/odb/source.h\nindex 7b8ff3d19d..4d13e4cfaf 100644\n--- a/odb/source.h\n+++ b/odb/source.h\n@@ -27,6 +27,7 @@ enum odb_source_type {\n \n struct object_id;\n struct odb_read_stream;\n+struct strbuf;\n struct strvec;\n \n /*\n@@ -111,12 +112,16 @@ struct odb_source {\n \t * already surfaced the object without reloading any on-disk state.\n \t *\n \t * The callback is expected to return an `enum odb_read_status`. Please\n-\t * refer to the individual values that can be returned.\n+\t * refer to the individual values that can be returned. In case reading\n+\t * the object has failed with a generic error and `errmsg` is non-NULL,\n+\t * the callback is expected to populate it with a human-readable\n+\t * message that describes the failure.\n \t */\n \tenum odb_read_status (*read_object_info)(struct odb_source *source,\n \t\t\t\t\t\t const struct object_id *oid,\n \t\t\t\t\t\t struct object_info *oi,\n-\t\t\t\t\t\t enum object_info_flags flags);\n+\t\t\t\t\t\t enum object_info_flags flags,\n+\t\t\t\t\t\t struct strbuf *errmsg);\n \n \t/*\n \t * This callback is expected to create a new read stream that can be\n@@ -341,13 +346,18 @@ static inline void odb_source_prepare(struct odb_source *source,\n /*\n * Read an object from the object database source identified by its object ID.\n * Please refer to `enum odb_read_status` for the individual error codes.\n+ *\n+ * In case reading the object has failed with a generic error and `errmsg` is\n+ * non-NULL it will be populated with a human-readable message that describes\n+ * the failure.\n */\n static inline enum odb_read_status odb_source_read_object_info(struct odb_source *source,\n \t\t\t\t\t\t\t const struct object_id *oid,\n \t\t\t\t\t\t\t struct object_info *oi,\n-\t\t\t\t\t\t\t enum object_info_flags flags)\n+\t\t\t\t\t\t\t enum object_info_flags flags,\n+\t\t\t\t\t\t\t struct strbuf *errmsg)\n {\n-\treturn source->read_object_info(source, oid, oi, flags);\n+\treturn source->read_object_info(source, oid, oi, flags, errmsg);\n }\n \n /*\ndiff --git a/packfile.c b/packfile.c\nindex 34e2f9bb8b..3cde39a01c 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1945,7 +1945,7 @@ int has_object_pack(struct repository *r, const struct object_id *oid)\n \todb_prepare_alternates(r->objects);\n \tfor (source = r->objects->sources; source; source = source->next) {\n \t\tstruct odb_source_files *files = odb_source_files_downcast(source);\n-\t\tif (!odb_source_read_object_info(&files->packed->base, oid, NULL, 0))\n+\t\tif (!odb_source_read_object_info(&files->packed->base, oid, NULL, 0, NULL))\n \t\t\treturn 1;\n \t}\n \ndiff --git a/t/unit-tests/u-odb-inmemory.c b/t/unit-tests/u-odb-inmemory.c\nindex 3e5068080c..095c20ba91 100644\n--- a/t/unit-tests/u-odb-inmemory.c\n+++ b/t/unit-tests/u-odb-inmemory.c\n@@ -29,7 +29,7 @@ static void cl_assert_object_info(struct odb_source_inmemory *source,\n \t\t.contentp = &actual_content,\n \t};\n \n-\tcl_must_pass(odb_source_read_object_info(&source->base, oid, &oi, 0));\n+\tcl_must_pass(odb_source_read_object_info(&source->base, oid, &oi, 0, NULL));\n \tcl_assert_equal_u(actual_size, strlen(expected_content));\n \tcl_assert_equal_u(actual_type, expected_type);\n \tcl_assert_equal_s((char *) actual_content, expected_content);\n@@ -72,7 +72,7 @@ void test_odb_inmemory__read_missing_object(void)\n \tconst char *end;\n \n \tcl_must_pass(parse_oid_hex_algop(RANDOM_OID, &oid, &end, repo.hash_algo));\n-\tcl_assert_equal_i(odb_source_read_object_info(&source->base, &oid, NULL, 0),\n+\tcl_assert_equal_i(odb_source_read_object_info(&source->base, &oid, NULL, 0, NULL),\n \t\t\t ODB_READ_NOT_FOUND);\n \n \todb_source_free(&source->base);\n\n-- \n2.55.0.822.g20453c30eb.dirty\n\n"},{"id":"550808","messageId":"20260819-pks-odb-generic-corrupt-objects-v2-5-a984e3a0ad6f@pks.im","threadId":"66194","inReplyTo":"20260819-pks-odb-generic-corrupt-objects-v2-0-a984e3a0ad6f@pks.im","subject":"[PATCH v2 5/5] odb: handle `OBJECT_INFO_DIE_IF_CORRUPT` generically","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-19T12:17:23Z","receivedAt":"2026-08-19T12:18:03Z","isPatch":true,"body":"When a lookup with `OBJECT_INFO_DIE_IF_CORRUPT` fails we want to die in\ncase the object exists, but cannot be read. This flag is handled in two\ndifferent spots right now:\n\n - `do_oid_object_info_extended()` calls `has_packed_and_bad()` to\n check whether the object is known to be corrupt in any packfile.\n This function reaches into the internals of the packed source and\n thus breaks the abstraction provided by our object sources.\n\n - The loose source handles the flag itself and dies directly in\n `read_object_info_from_path()`, which means that we die even in\n cases where another source may still have a good copy of the\n object.\n\nBesides being inconsistent, it also ties us to the specific backend used\nby the database sources because `has_packed_and_bad()` assumes that they\nuse the \"files\" backend. Any other backend will instead cause us to die\nwhen calling `odb_source_files_downcast()`, even if the object was\nsimply nonexistent.\n\nIn the preceding commits we've carved out the infrastructure to make\nthis mechanism fully generic. On the one hand, all backends now tell us\nwhether the object is missing or corrupt via their return values. And\non the other hand, they have been taught to provide a readable error\nmessage to the caller.\n\nAdapt `do_oid_object_info_extended()` to use those new mechanisms. This\nmeans that we won't die immediately anymore when a loose object is\ncorrupt, and we properly handle backends other than the \"files\" backend.\n\nSigned-off-by: Patrick Steinhardt \n---\n odb.c | 46 ++++++++++++++++++++++++++++++--------------\n odb/source-loose.c | 10 ++--------\n packfile.c | 17 ----------------\n packfile.h | 1 -\n t/t1060-object-corruption.sh | 18 +++++++++++++++++\n 5 files changed, 52 insertions(+), 40 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex 83a53f7f6b..6bbea64033 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -15,7 +15,6 @@\n #include \"object-name.h\"\n #include \"odb.h\"\n #include \"odb/source-inmemory.h\"\n-#include \"packfile.h\"\n #include \"path.h\"\n #include \"promisor-remote.h\"\n #include \"quote.h\"\n@@ -551,8 +550,11 @@ static enum odb_read_status do_oid_object_info_extended(struct object_database *\n \t\t\t\t\t\t\tconst struct object_id *oid,\n \t\t\t\t\t\t\tstruct object_info *oi, unsigned flags)\n {\n+\tstruct strbuf corrupt_err = STRBUF_INIT;\n \tconst struct object_id *real = oid;\n+\tenum odb_read_status ret;\n \tint already_retried = 0;\n+\tbool corrupt = false;\n \n \tif (flags & OBJECT_INFO_LOOKUP_REPLACE)\n \t\treal = lookup_replace_object(odb->repo, oid);\n@@ -568,9 +570,14 @@ static enum odb_read_status do_oid_object_info_extended(struct object_database *\n \twhile (1) {\n \t\tstruct odb_source *source;\n \n-\t\tfor (source = odb->sources; source; source = source->next)\n-\t\t\tif (!odb_source_read_object_info(source, real, oi, flags, NULL))\n-\t\t\t\treturn 0;\n+\t\tfor (source = odb->sources; source; source = source->next) {\n+\t\t\tret = odb_source_read_object_info(source, real, oi, flags,\n+\t\t\t\t\t\t\t corrupt_err.len ? NULL : &corrupt_err);\n+\t\t\tif (!ret)\n+\t\t\t\tgoto out;\n+\t\t\tif (ret != ODB_READ_NOT_FOUND)\n+\t\t\t\tcorrupt = true;\n+\t\t}\n \n \t\t/*\n \t\t * When the object hasn't been found we try a second read and\n@@ -578,11 +585,15 @@ static enum odb_read_status do_oid_object_info_extended(struct object_database *\n \t\t * caches or reload on-disk state.\n \t\t */\n \t\tif (!(flags & OBJECT_INFO_QUICK)) {\n-\t\t\tfor (source = odb->sources; source; source = source->next)\n-\t\t\t\tif (!odb_source_read_object_info(source, real, oi,\n-\t\t\t\t\t\t\t\t flags | OBJECT_INFO_SECOND_READ,\n-\t\t\t\t\t\t\t\t NULL))\n-\t\t\t\t\treturn 0;\n+\t\t\tfor (source = odb->sources; source; source = source->next) {\n+\t\t\t\tret = odb_source_read_object_info(source, real, oi,\n+\t\t\t\t\t\t\t\t flags | OBJECT_INFO_SECOND_READ,\n+\t\t\t\t\t\t\t\t corrupt_err.len ? NULL : &corrupt_err);\n+\t\t\t\tif (!ret)\n+\t\t\t\t\tgoto out;\n+\t\t\t\tif (ret != ODB_READ_NOT_FOUND)\n+\t\t\t\t\tcorrupt = true;\n+\t\t\t}\n \t\t}\n \n \t\t/*\n@@ -605,16 +616,23 @@ static enum odb_read_status do_oid_object_info_extended(struct object_database *\n \t\t}\n \n \t\tif (flags & OBJECT_INFO_DIE_IF_CORRUPT) {\n-\t\t\tconst struct packed_git *p;\n \t\t\tif ((flags & OBJECT_INFO_LOOKUP_REPLACE) && !oideq(real, oid))\n \t\t\t\tdie(_(\"replacement %s not found for %s\"),\n \t\t\t\t oid_to_hex(real), oid_to_hex(oid));\n-\t\t\tif ((p = has_packed_and_bad(odb->repo, real)))\n-\t\t\t\tdie(_(\"packed object %s (stored in %s) is corrupt\"),\n-\t\t\t\t oid_to_hex(real), p->pack_name);\n+\t\t\tif (corrupt) {\n+\t\t\t\tif (corrupt_err.len)\n+\t\t\t\t\tdie(\"%s\", corrupt_err.buf);\n+\t\t\t\tdie(_(\"object %s is corrupt\"), oid_to_hex(real));\n+\t\t\t}\n \t\t}\n-\t\treturn -1;\n+\n+\t\tret = corrupt ? ODB_READ_ERROR : ODB_READ_NOT_FOUND;\n+\t\tgoto out;\n \t}\n+\n+out:\n+\tstrbuf_release(&corrupt_err);\n+\treturn ret;\n }\n \n static int oid_object_info_convert(struct repository *r,\ndiff --git a/odb/source-loose.c b/odb/source-loose.c\nindex b57ee2701a..540b2dd40d 100644\n--- a/odb/source-loose.c\n+++ b/odb/source-loose.c\n@@ -192,15 +192,9 @@ static int read_object_info_from_path(struct odb_source_loose *loose,\n \tret = 0;\n \n out:\n-\tif (ret && ret != ODB_READ_NOT_FOUND) {\n-\t\tif ((flags & OBJECT_INFO_DIE_IF_CORRUPT))\n-\t\t\tdie(_(\"loose object %s (stored in %s) is corrupt\"),\n+\tif (ret && ret != ODB_READ_NOT_FOUND && errmsg)\n+\t\tstrbuf_addf(errmsg, _(\"loose object %s (stored in %s) is corrupt\"),\n \t\t\t oid_to_hex(oid), path);\n-\t\tif (errmsg)\n-\t\t\tstrbuf_addf(errmsg, _(\"loose object %s (stored in %s) is corrupt\"),\n-\t\t\t\t oid_to_hex(oid), path);\n-\t}\n-\n \tif (stream_to_end)\n \t\tgit_inflate_end(stream_to_end);\n \tif (map)\ndiff --git a/packfile.c b/packfile.c\nindex 3cde39a01c..cd38be088d 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -985,23 +985,6 @@ void mark_bad_packed_object(struct packed_git *p, const struct object_id *oid)\n \toidset_insert(&p->bad_objects, oid);\n }\n \n-const struct packed_git *has_packed_and_bad(struct repository *r,\n-\t\t\t\t\t const struct object_id *oid)\n-{\n-\tstruct odb_source *source;\n-\n-\tfor (source = r->objects->sources; source; source = source->next) {\n-\t\tstruct odb_source_files *files = odb_source_files_downcast(source);\n-\t\tstruct packfile_list_entry *e;\n-\n-\t\tfor (e = files->packed->packs.head; e; e = e->next)\n-\t\t\tif (oidset_contains(&e->pack->bad_objects, oid))\n-\t\t\t\treturn e->pack;\n-\t}\n-\n-\treturn NULL;\n-}\n-\n off_t get_delta_base(struct packed_git *p,\n \t\t struct pack_window **w_curs,\n \t\t off_t *curpos,\ndiff --git a/packfile.h b/packfile.h\nindex 3229a6ed47..573fe003d0 100644\n--- a/packfile.h\n+++ b/packfile.h\n@@ -329,7 +329,6 @@ int packed_object_info_with_index_pos(struct odb_source_packed *source,\n \t\t\t\t uint32_t *maybe_index_pos, struct object_info *oi);\n \n void mark_bad_packed_object(struct packed_git *, const struct object_id *);\n-const struct packed_git *has_packed_and_bad(struct repository *, const struct object_id *);\n \n int has_object_pack(struct repository *r, const struct object_id *oid);\n int has_object_kept_pack(struct repository *r, const struct object_id *oid,\ndiff --git a/t/t1060-object-corruption.sh b/t/t1060-object-corruption.sh\nindex 502a5ea1c5..d2ef468b45 100755\n--- a/t/t1060-object-corruption.sh\n+++ b/t/t1060-object-corruption.sh\n@@ -145,4 +145,22 @@ test_expect_success 'partial clone of corrupted repository' '\n \ttest_must_fail git -C corrupt-partial checkout --force\n '\n \n+test_expect_success 'corrupted loose commit can be read from alternate' '\n+\tgit init repo-a &&\n+\ttree=$(git -C repo-a write-tree) &&\n+\tcommit=$(git -C repo-a commit-tree $tree .git/objects/info/alternates &&\n+\t\tcorrupt_byte \"$commit\" 1\n+\t) &&\n+\n+\tgit -C repo-a cat-file -p \"$commit\" >expect &&\n+\tgit -C repo-b cat-file -p \"$commit\" >actual 2>err &&\n+\ttest_cmp expect actual &&\n+\ttest_grep \"inflate: data stream error\" err\n+'\n+\n test_done\n\n-- \n2.55.0.822.g20453c30eb.dirty\n\n"},{"id":"550897","messageId":"CAOLa=ZSCf3CvTwtgj7RXncT6zPhyp4EX9r=g55uD+mTA1zp-5w@mail.gmail.com","threadId":"66194","inReplyTo":"20260819-pks-odb-generic-corrupt-objects-v2-2-a984e3a0ad6f@pks.im","subject":"Re: [PATCH v2 2/5] odb/source: introduce error status when reading objects","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-08-20T12:41:10Z","receivedAt":"2026-08-20T12:41:12Z","isPatch":true,"body":"Patrick Steinhardt writes:\n\n> The `read_object_info()` callback of `struct odb_source` is documented\n> to return a negative error code in case reading the object has failed,\n> and zero otherwise. This is overly broad though, as there are two very\n> different kinds of failures:\n>\n> - The object may not exist in the source at all.\n>\n> - The object exists, but reading it has failed, for example because\n> its on-disk state is corrupt.\n>\n> This distinction matters to callers: when an object is corrupt in one\n> source we may still find a good copy of it in another source, so we may\n> still be able to proceed with a given operation.\n>\n\nBut isn't that the same for an object not existing in a source? If it\ndoesn't exist in one source, we may find a good copy of it in another?\n\n> The \"packed\" source already distinguishes these cases by returning a\n> positive value for missing objects and a negative value in case reading\n> the object has failed. But it is the only such source that distinguishes\n> those cases, and the returned value is translated into a negative error\n> code by the \"files\" backend anyway.\n>\n> Introduce a new error status that is specific to reading objects and\n> adapt the infrastructure to return it. For now, we only discern\n> successful reads from generic failures, which mostly matches the status\n> quo. In subsequent commits though we're about to add an error that\n> explicitly tells the caller that an object does not exist.\n>\n> Note that we keep the \"packed\" backend as-is with its positive return\n> code for missing objects. This will be fixed in the next commit.\n>\n> Signed-off-by: Patrick Steinhardt \n> ---\n> odb.c | 16 ++++++++--------\n> odb.h | 15 +++++++++++----\n> odb/source-files.c | 8 ++++----\n> odb/source-inmemory.c | 8 ++++----\n> odb/source-loose.c | 8 ++++----\n> odb/source-packed.c | 8 ++++----\n> odb/source.h | 22 +++++++++++-----------\n> 7 files changed, 46 insertions(+), 39 deletions(-)\n>\n> diff --git a/odb.c b/odb.c\n> index caf1d0f542..1b37b26376 100644\n> --- a/odb.c\n> +++ b/odb.c\n> @@ -547,9 +547,9 @@ static int register_all_submodule_sources(struct object_database *odb)\n> \treturn ret;\n> }\n>\n> -static int do_oid_object_info_extended(struct object_database *odb,\n> -\t\t\t\t const struct object_id *oid,\n> -\t\t\t\t struct object_info *oi, unsigned flags)\n> +static enum odb_read_status do_oid_object_info_extended(struct object_database *odb,\n> +\t\t\t\t\t\t\tconst struct object_id *oid,\n> +\t\t\t\t\t\t\tstruct object_info *oi, unsigned flags)\n> {\n> \tconst struct object_id *real = oid;\n> \tint already_retried = 0;\n> @@ -696,12 +696,12 @@ static int oid_object_info_convert(struct repository *r,\n> \treturn ret;\n> }\n>\n\nHere and elsewhere. Shouldn't we explicitly return ODB_READ_OK or\nODB_READ_ERROR instead of relying on implicit conversion?\n\n[snip]\n"},{"id":"550898","messageId":"CAOLa=ZSSzR+qKh4Do-F7xZQMO-pE+t4N8qM5hsbfM4Uh7i3d1A@mail.gmail.com","threadId":"66194","inReplyTo":"20260819-pks-odb-generic-corrupt-objects-v2-3-a984e3a0ad6f@pks.im","subject":"Re: [PATCH v2 3/5] odb/source: let callers discern missing and corrupt objects","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-08-20T12:56:50Z","receivedAt":"2026-08-20T12:56:52Z","isPatch":true,"body":"Patrick Steinhardt writes:\n\n> As explained in the preceding commits, reading objects can either fail\n> because the object truly does not exist or because it exists, but its\n> data is corrupt. Some callers do care about this distinction, but there\n> is no way to tell these two cases apart right now.\n>\n> Introduce a new `ODB_READ_NOT_FOUND` value that ought to be returned by\n> the backends in case the object truly does not exist and adapt backends\n> to use it.\n>\n> Note that we don't yet return this error from `odb_read_object_info()`\n> itself. This will be fixed in a subsequent commit.\n>\n> Signed-off-by: Patrick Steinhardt \n> ---\n> odb.h | 2 ++\n> odb/source-files.c | 20 +++++++++++++++++---\n> odb/source-inmemory.c | 2 +-\n> odb/source-loose.c | 31 +++++++++++++++++++------------\n> odb/source-packed.c | 2 +-\n> t/unit-tests/u-odb-inmemory.c | 3 ++-\n> 6 files changed, 42 insertions(+), 18 deletions(-)\n>\n> diff --git a/odb.h b/odb.h\n> index 43cbcc3aba..1264d4ce7d 100644\n> --- a/odb.h\n> +++ b/odb.h\n> @@ -440,6 +440,8 @@ enum odb_read_status {\n> \tODB_READ_OK = 0,\n> \t/* The read resulted in a generic error. */\n> \tODB_READ_ERROR = -1,\n> +\t/* The object could not be found. */\n> +\tODB_READ_NOT_FOUND = -2,\n> };\n>\n> /*\n> diff --git a/odb/source-files.c b/odb/source-files.c\n> index a28aa5042d..e88fd1d399 100644\n> --- a/odb/source-files.c\n> +++ b/odb/source-files.c\n> @@ -65,12 +65,26 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source\n> \t\t\t\t\t\t\t enum object_info_flags flags)\n> {\n> \tstruct odb_source_files *files = odb_source_files_downcast(source);\n> +\tenum odb_read_status ret_packed, ret_loose;\n>\n> -\tif (!odb_source_read_object_info(&files->packed->base, oid, oi, flags) ||\n> -\t !odb_source_read_object_info(&files->loose->base, oid, oi, flags))\n> +\tret_packed = odb_source_read_object_info(&files->packed->base, oid, oi, flags);\n> +\tif (!ret_packed)\n> \t\treturn 0;\n>\n\nNit: Similar to my previous comment, wouldn't it be nicer to do\n\n if (ret_packed == ODB_READ_OK)\n return 0;\n\n\n> -\treturn -1;\n> +\tret_loose = odb_source_read_object_info(&files->loose->base, oid, oi, flags);\n> +\tif (!ret_loose)\n> +\t\treturn 0;\n> +\n> +\t/*\n> +\t * Reading the packed object may have failed even though the object\n> +\t * exists, for example because it is corrupt. Report this failure to\n> +\t * the caller in case neither of the sources was able to read the\n> +\t * object, and prefer the error of the packed source in case both\n> +\t * reads have failed.\n> +\t */\n> +\tif (ret_packed != ODB_READ_NOT_FOUND)\n> +\t\treturn ret_packed;\n> +\treturn ret_loose;\n> }\n>\n\nSo if we already found the source we return early and only come here for\nerrors. What I don't understand is why we filter out ODB_READ_NOT_FOUND\nfor packed. Wouldn't that leave us with\n\n ret_packed => ODB_READ_ERROR\n ret_loose => ODB_READ_ERROR or ODB_READ_NOT_FOUND\n\nDoesn't this come down to preferring to propagate ODB_READ_NOT_FOUND over\nODB_READ_ERROR and now packed error over loose?\n\n[snip]\n\nThe rest look in order.\n"},{"id":"550904","messageId":"CAOLa=ZTVxdVAJynKjb0LjmZ-+b5nQmyD0Bm-aT81rOOdJ0a5yg@mail.gmail.com","threadId":"66194","inReplyTo":"20260819-pks-odb-generic-corrupt-objects-v2-0-a984e3a0ad6f@pks.im","subject":"Re: [PATCH v2 0/5] odb: handle `OBJECT_INFO_DIE_IF_CORRUPT` generically","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-08-20T14:14:59Z","receivedAt":"2026-08-20T14:15:02Z","isPatch":true,"body":"Patrick Steinhardt writes:\n\n> Hi,\n>\n> when looking up an object with `OBJECT_INFO_DIE_IF_CORRUPT` fails we\n> want to die in case the object exists but is corrupted. This flag is\n> handled in two different spots right now:\n>\n> - `do_oid_object_info_extended()` calls `has_packed_and_bad()` to\n> check whether the object is known to be corrupt in any packfile.\n> This function reaches into the internals of the packed source and\n> thus breaks the abstraction provided by our object sources.\n>\n> - The loose source handles the flag itself and dies directly in\n> `read_object_info_from_path()`, which means that we die even in\n> cases where another source may still have a good copy of the\n> object.\n>\n> Besides being inconsistent, it also ties us to the specific backend used\n> by the database sources because `has_packed_and_bad()` assumes that they\n> use the \"files\" backend. Any other backend will instead cause us to die\n> when calling `odb_source_files_downcast()`, even if the object was\n> simply nonexistent.\n>\n> This series fixes these issues and makes the check backend-agnostic by\n> extending semantics of `odb_source_read_object_info()`: on the one hand\n> it now distinguishes whether an object is missing or corrput, and on the\n> other hand it starts to return an error message to the caller.\n>\n> Changes in v2:\n> - Adapt the series to use an `enum odb_read_status` with negative\n> error codes exclusively, as suggested by Junio. This results in a\n> rather big restructure of the series.\n> - Link to v1: https://patch.msgid.link/20260818-pks-odb-generic-corrupt-objects-v1-0-ec234567510f@pks.im\n>\n\nSmall questions from me, looks good otherwise! :)\n\n[snip]\n"},{"id":"550905","messageId":"aocNq1N9MWS4BeaJ@pks.im","threadId":"66194","inReplyTo":"CAOLa=ZSCf3CvTwtgj7RXncT6zPhyp4EX9r=g55uD+mTA1zp-5w@mail.gmail.com","subject":"Re: [PATCH v2 2/5] odb/source: introduce error status when reading objects","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-20T14:22:35Z","receivedAt":"2026-08-20T14:22:43Z","isPatch":true,"body":"On Thu, Aug 20, 2026 at 08:41:10AM -0400, Karthik Nayak wrote:\n> Patrick Steinhardt writes:\n> \n> > The `read_object_info()` callback of `struct odb_source` is documented\n> > to return a negative error code in case reading the object has failed,\n> > and zero otherwise. This is overly broad though, as there are two very\n> > different kinds of failures:\n> >\n> > - The object may not exist in the source at all.\n> >\n> > - The object exists, but reading it has failed, for example because\n> > its on-disk state is corrupt.\n> >\n> > This distinction matters to callers: when an object is corrupt in one\n> > source we may still find a good copy of it in another source, so we may\n> > still be able to proceed with a given operation.\n> >\n> \n> But isn't that the same for an object not existing in a source? If it\n> doesn't exist in one source, we may find a good copy of it in another?\n\nYeah, that paragraph is a bit odd indeed. What I really wanted to say is\nthat the failure mode is different depending on whether the object is\nfound at all: if it's not then we'd fail gracefully, if it is but it's\ncorrupt then we die.\n\n> > diff --git a/odb.c b/odb.c\n> > index caf1d0f542..1b37b26376 100644\n> > --- a/odb.c\n> > +++ b/odb.c\n> > @@ -696,12 +696,12 @@ static int oid_object_info_convert(struct repository *r,\n> > \treturn ret;\n> > }\n> >\n> \n> Here and elsewhere. Shouldn't we explicitly return ODB_READ_OK or\n> ODB_READ_ERROR instead of relying on implicit conversion?\n\nI didn't want to go through the complete callchain to make sure that we\nexplicitly return those values. I think it'd be mostly pointless: the\nreturn code convention is established enough, and all callers already\nreturn the expected values anyway, even though they're not using the\nenum now.\n\nPatrick\n"},{"id":"550906","messageId":"aocNsR60-8W2A-fy@pks.im","threadId":"66194","inReplyTo":"CAOLa=ZSSzR+qKh4Do-F7xZQMO-pE+t4N8qM5hsbfM4Uh7i3d1A@mail.gmail.com","subject":"Re: [PATCH v2 3/5] odb/source: let callers discern missing and corrupt objects","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-20T14:22:41Z","receivedAt":"2026-08-20T14:22:46Z","isPatch":true,"body":"On Thu, Aug 20, 2026 at 08:56:50AM -0400, Karthik Nayak wrote:\n> Patrick Steinhardt writes:\n> > diff --git a/odb/source-files.c b/odb/source-files.c\n> > index a28aa5042d..e88fd1d399 100644\n> > --- a/odb/source-files.c\n> > +++ b/odb/source-files.c\n> > @@ -65,12 +65,26 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source\n> > \t\t\t\t\t\t\t enum object_info_flags flags)\n> > {\n> > \tstruct odb_source_files *files = odb_source_files_downcast(source);\n> > +\tenum odb_read_status ret_packed, ret_loose;\n> >\n> > -\tif (!odb_source_read_object_info(&files->packed->base, oid, oi, flags) ||\n> > -\t !odb_source_read_object_info(&files->loose->base, oid, oi, flags))\n> > +\tret_packed = odb_source_read_object_info(&files->packed->base, oid, oi, flags);\n> > +\tif (!ret_packed)\n> > \t\treturn 0;\n> >\n> \n> Nit: Similar to my previous comment, wouldn't it be nicer to do\n> \n> if (ret_packed == ODB_READ_OK)\n> return 0;\n\nAs mentioned in the preceding commit, I think it would be somewhat\npointless and only make the code more verbose without much of a purpose.\n\n> > -\treturn -1;\n> > +\tret_loose = odb_source_read_object_info(&files->loose->base, oid, oi, flags);\n> > +\tif (!ret_loose)\n> > +\t\treturn 0;\n> > +\n> > +\t/*\n> > +\t * Reading the packed object may have failed even though the object\n> > +\t * exists, for example because it is corrupt. Report this failure to\n> > +\t * the caller in case neither of the sources was able to read the\n> > +\t * object, and prefer the error of the packed source in case both\n> > +\t * reads have failed.\n> > +\t */\n> > +\tif (ret_packed != ODB_READ_NOT_FOUND)\n> > +\t\treturn ret_packed;\n> > +\treturn ret_loose;\n> > }\n> >\n> \n> So if we already found the source we return early and only come here for\n> errors. What I don't understand is why we filter out ODB_READ_NOT_FOUND\n> for packed. Wouldn't that leave us with\n> \n> ret_packed => ODB_READ_ERROR\n> ret_loose => ODB_READ_ERROR or ODB_READ_NOT_FOUND\n> \n> Doesn't this come down to preferring to propagate ODB_READ_NOT_FOUND over\n> ODB_READ_ERROR and now packed error over loose?\n\nSo here we know that we didn't find the object. So there's four cases:\n\n - The object was not found in either, and we'll return\n ODB_READ_NOT_FOUND.\n\n - The object was not found in the \"packed\" source but was found in the\n \"loose\" source. So we'd have `ret_packed == ODB_READ_NOT_FOUND` and\n `ret_loose` at any other error code. And consequently this block:\n\n if (ret_packed != ODB_READ_NOT_FOUND)\n return ret_packed;\n\n Would not trigger as `ret_packed` _is_ ODB_READ_NOT_FOUND. Hence, we\n favor the error from `ret_loose`, which contains our corruption\n error.\n\n - The reverse case, where the object exists in the \"packed\" backend\n but is corrupt. In that case `ret_packed != ODB_READ_NOT_FOUND`\n evaluates true, and we bubble up that error.\n\n - Both sources have a corrupt object. If so, we simply favor the\n packed error because we have to pick one.\n\nI think you've simply misread the condition, as we do exactly the\nreverse.\n\nPatrick\n"},{"id":"550941","messageId":"CAOLa=ZTKqPa-9j8UzqQQCVwVk=Yt7aQVxg9qSi41Q+=A-6dAow@mail.gmail.com","threadId":"66194","inReplyTo":"aocNq1N9MWS4BeaJ@pks.im","subject":"Re: [PATCH v2 2/5] odb/source: introduce error status when reading objects","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-08-20T20:59:48Z","receivedAt":"2026-08-20T20:59:49Z","isPatch":true,"body":"Patrick Steinhardt writes:\n\n> On Thu, Aug 20, 2026 at 08:41:10AM -0400, Karthik Nayak wrote:\n>> Patrick Steinhardt writes:\n>>\n>> > The `read_object_info()` callback of `struct odb_source` is documented\n>> > to return a negative error code in case reading the object has failed,\n>> > and zero otherwise. This is overly broad though, as there are two very\n>> > different kinds of failures:\n>> >\n>> > - The object may not exist in the source at all.\n>> >\n>> > - The object exists, but reading it has failed, for example because\n>> > its on-disk state is corrupt.\n>> >\n>> > This distinction matters to callers: when an object is corrupt in one\n>> > source we may still find a good copy of it in another source, so we may\n>> > still be able to proceed with a given operation.\n>> >\n>>\n>> But isn't that the same for an object not existing in a source? If it\n>> doesn't exist in one source, we may find a good copy of it in another?\n>\n> Yeah, that paragraph is a bit odd indeed. What I really wanted to say is\n> that the failure mode is different depending on whether the object is\n> found at all: if it's not then we'd fail gracefully, if it is but it's\n> corrupt then we die.\n>\n>> > diff --git a/odb.c b/odb.c\n>> > index caf1d0f542..1b37b26376 100644\n>> > --- a/odb.c\n>> > +++ b/odb.c\n>> > @@ -696,12 +696,12 @@ static int oid_object_info_convert(struct repository *r,\n>> > \treturn ret;\n>> > }\n>> >\n>>\n>> Here and elsewhere. Shouldn't we explicitly return ODB_READ_OK or\n>> ODB_READ_ERROR instead of relying on implicit conversion?\n>\n> I didn't want to go through the complete callchain to make sure that we\n> explicitly return those values. I think it'd be mostly pointless: the\n> return code convention is established enough, and all callers already\n> return the expected values anyway, even though they're not using the\n> enum now.\n>\n> Patrick\n\nI think logically it is correct already, but returning an enum type but\nseeing -1,0,1 in the return statements means that we have to either\nremember the different enum values or we need to cross reference each\ntime. Eventually someone would start using 'return ODB_READ_OK' and so\non and then we'd have a mix of both (this argument does go both way).\nAnyway, it is fine as is.\n"},{"id":"550946","messageId":"CAOLa=ZSs-9VU2eKT8DUJ7FzZCAkgRzZ6_XQZBP=x7avxpFp7qw@mail.gmail.com","threadId":"66194","inReplyTo":"aocNsR60-8W2A-fy@pks.im","subject":"Re: [PATCH v2 3/5] odb/source: let callers discern missing and corrupt objects","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-08-20T21:09:51Z","receivedAt":"2026-08-20T21:09:53Z","isPatch":true,"body":"Patrick Steinhardt writes:\n\n> On Thu, Aug 20, 2026 at 08:56:50AM -0400, Karthik Nayak wrote:\n>> Patrick Steinhardt writes:\n>> > diff --git a/odb/source-files.c b/odb/source-files.c\n>> > index a28aa5042d..e88fd1d399 100644\n>> > --- a/odb/source-files.c\n>> > +++ b/odb/source-files.c\n>> > @@ -65,12 +65,26 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source\n>> > \t\t\t\t\t\t\t enum object_info_flags flags)\n>> > {\n>> > \tstruct odb_source_files *files = odb_source_files_downcast(source);\n>> > +\tenum odb_read_status ret_packed, ret_loose;\n>> >\n>> > -\tif (!odb_source_read_object_info(&files->packed->base, oid, oi, flags) ||\n>> > -\t !odb_source_read_object_info(&files->loose->base, oid, oi, flags))\n>> > +\tret_packed = odb_source_read_object_info(&files->packed->base, oid, oi, flags);\n>> > +\tif (!ret_packed)\n>> > \t\treturn 0;\n>> >\n>>\n>> Nit: Similar to my previous comment, wouldn't it be nicer to do\n>>\n>> if (ret_packed == ODB_READ_OK)\n>> return 0;\n>\n> As mentioned in the preceding commit, I think it would be somewhat\n> pointless and only make the code more verbose without much of a purpose.\n>\n>> > -\treturn -1;\n>> > +\tret_loose = odb_source_read_object_info(&files->loose->base, oid, oi, flags);\n>> > +\tif (!ret_loose)\n>> > +\t\treturn 0;\n>> > +\n>> > +\t/*\n>> > +\t * Reading the packed object may have failed even though the object\n>> > +\t * exists, for example because it is corrupt. Report this failure to\n>> > +\t * the caller in case neither of the sources was able to read the\n>> > +\t * object, and prefer the error of the packed source in case both\n>> > +\t * reads have failed.\n>> > +\t */\n>> > +\tif (ret_packed != ODB_READ_NOT_FOUND)\n>> > +\t\treturn ret_packed;\n>> > +\treturn ret_loose;\n>> > }\n>> >\n>>\n>> So if we already found the source we return early and only come here for\n>> errors. What I don't understand is why we filter out ODB_READ_NOT_FOUND\n>> for packed. Wouldn't that leave us with\n>>\n>> ret_packed => ODB_READ_ERROR\n>> ret_loose => ODB_READ_ERROR or ODB_READ_NOT_FOUND\n>>\n>> Doesn't this come down to preferring to propagate ODB_READ_NOT_FOUND over\n>> ODB_READ_ERROR and now packed error over loose?\n>\n> So here we know that we didn't find the object. So there's four cases:\n>\n> - The object was not found in either, and we'll return\n> ODB_READ_NOT_FOUND.\n>\n> - The object was not found in the \"packed\" source but was found in the\n> \"loose\" source. So we'd have `ret_packed == ODB_READ_NOT_FOUND` and\n> `ret_loose` at any other error code. And consequently this block:\n>\n> if (ret_packed != ODB_READ_NOT_FOUND)\n> return ret_packed;\n>\n> Would not trigger as `ret_packed` _is_ ODB_READ_NOT_FOUND. Hence, we\n> favor the error from `ret_loose`, which contains our corruption\n> error.\n>\n> - The reverse case, where the object exists in the \"packed\" backend\n> but is corrupt. In that case `ret_packed != ODB_READ_NOT_FOUND`\n> evaluates true, and we bubble up that error.\n>\n> - Both sources have a corrupt object. If so, we simply favor the\n> packed error because we have to pick one.\n>\n> I think you've simply misread the condition, as we do exactly the\n> reverse.\n>\n> Patrick\n\nOops. Thanks for the detailed response.\n\nI think I made my case in reverse, but my original argument still\nholds.\n\nret_packed ret_loose ret_packed != NOT_FOUND ? returned\n----------- ----------- --------------------------- -----------------\nNOT_FOUND NOT_FOUND false ret_loose (NOT_FOUND)\nNOT_FOUND ERROR false ret_loose (ERROR)\nERROR NOT_FOUND true ret_packed (ERROR)\nERROR ERROR true ret_packed (ERROR)\n\nSo since we return ret_loose as many times as ret_packed. The comment:\n\n> and prefer the error of the packed source in case both reads have\n> failed.\n\nisn't true entirely. So isn't it better modified to something like\n\"prefer other errors over not found errors\" or something. I hope that\nmakes sense?\n\n- Karthik\n"},{"id":"550981","messageId":"aofl8e4P6BqJaQEm@pks.im","threadId":"66194","inReplyTo":"CAOLa=ZSs-9VU2eKT8DUJ7FzZCAkgRzZ6_XQZBP=x7avxpFp7qw@mail.gmail.com","subject":"Re: [PATCH v2 3/5] odb/source: let callers discern missing and corrupt objects","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-21T05:45:21Z","receivedAt":"2026-08-21T05:45:28Z","isPatch":true,"body":"On Thu, Aug 20, 2026 at 05:09:51PM -0400, Karthik Nayak wrote:\n> Patrick Steinhardt writes:\n> > On Thu, Aug 20, 2026 at 08:56:50AM -0400, Karthik Nayak wrote:\n> Oops. Thanks for the detailed response.\n> \n> I think I made my case in reverse, but my original argument still\n> holds.\n> \n> ret_packed ret_loose ret_packed != NOT_FOUND ? returned\n> ----------- ----------- --------------------------- -----------------\n> NOT_FOUND NOT_FOUND false ret_loose (NOT_FOUND)\n> NOT_FOUND ERROR false ret_loose (ERROR)\n> ERROR NOT_FOUND true ret_packed (ERROR)\n> ERROR ERROR true ret_packed (ERROR)\n> \n> So since we return ret_loose as many times as ret_packed. The comment:\n> \n> > and prefer the error of the packed source in case both reads have\n> > failed.\n> \n> isn't true entirely. So isn't it better modified to something like\n> \"prefer other errors over not found errors\" or something. I hope that\n> makes sense?\n\nBut we don't. As your above table shows, we return errors twice from the\npacked backend and only once from the loose backend. And in case both\nsources returned an error, we prefer the packed one.\n\nI think where we're talking past one another is that I distinguish\nbetween errors (-1) and NOT_FOUND.\n\nPatrick\n"},{"id":"551013","messageId":"CAOLa=ZRPudL38f9wTZoRNXLPeEpX5OSC_kaQyG1xgyNNriruMQ@mail.gmail.com","threadId":"66194","inReplyTo":"aofl8e4P6BqJaQEm@pks.im","subject":"Re: [PATCH v2 3/5] odb/source: let callers discern missing and corrupt objects","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-08-21T12:39:54Z","receivedAt":"2026-08-21T12:39:56Z","isPatch":true,"body":"Patrick Steinhardt writes:\n\n> On Thu, Aug 20, 2026 at 05:09:51PM -0400, Karthik Nayak wrote:\n>> Patrick Steinhardt writes:\n>> > On Thu, Aug 20, 2026 at 08:56:50AM -0400, Karthik Nayak wrote:\n>> Oops. Thanks for the detailed response.\n>>\n>> I think I made my case in reverse, but my original argument still\n>> holds.\n>>\n>> ret_packed ret_loose ret_packed != NOT_FOUND ? returned\n>> ----------- ----------- --------------------------- -----------------\n>> NOT_FOUND NOT_FOUND false ret_loose (NOT_FOUND)\n>> NOT_FOUND ERROR false ret_loose (ERROR)\n>> ERROR NOT_FOUND true ret_packed (ERROR)\n>> ERROR ERROR true ret_packed (ERROR)\n>>\n>> So since we return ret_loose as many times as ret_packed. The comment:\n>>\n>> > and prefer the error of the packed source in case both reads have\n>> > failed.\n>>\n>> isn't true entirely. So isn't it better modified to something like\n>> \"prefer other errors over not found errors\" or something. I hope that\n>> makes sense?\n>\n> But we don't. As your above table shows, we return errors twice from the\n> packed backend and only once from the loose backend. And in case both\n> sources returned an error, we prefer the packed one.\n>\n> I think where we're talking past one another is that I distinguish\n> between errors (-1) and NOT_FOUND.\n>\n> Patrick\n\nYeah I figured, plus this isn't significant. Thanks for humoring me :)\n"}]}