# [PATCH] builtin/cat-file.c: simplify calling `report_object_status()` 2 messages from 2025-10-13 to 2025-10-14. Participants: Taylor Blau, Jeff King. Thread: https://gitlist.dev/t/64313 ## Taylor Blau, 2025-10-13 21:56 Subject: [PATCH] builtin/cat-file.c: simplify calling `report_object_status()` Message-ID: <9236fa6e654a59337266430a2a8807da21210cd4.1760392502.git.me@ttaylorr.com> URL: https://gitlist.dev/e/9236fa6e654a59337266430a2a8807da21210cd4.1760392502.git.me%40ttaylorr.com ``` In b0b910e052 (cat-file.c: add batch handling for submodules, 2025-06-02), we began handling submodule entries specially when batching cat-file like so: $ echo :sha1collisiondetection | git.compile cat-file --batch-check 855827c583bc30645ba427885caa40c5b81764d2 submodule Commit b0b910e052 notes that submodules are handled differently than non-existent objects, which print " ", since there is (a) no object to resolve the OID of in the first place, and as commit b0b910e052 notes, (b) for submodules in particular, it is useful to know what commit it points at without having to spawn another Git process. That commit does so by calling report_object_status() and passing in "oid_to_hex(&data->oid)" for the "obj_name" parameter. This is unnecessary, however, since report_object_status() will do the same automatically if given a NULL "obj_name" argument. That behavior dates back to 6a951937ae (cat-file: add --batch-all-objects option, 2015-06-22), so rely on that instead of having the caller open-code that part of report_object_status(). Signed-off-by: Taylor Blau --- I noticed this while merging v2.50.1 into GitHub's private fork, and thought it was a good opportunity for some light clean-up. builtin/cat-file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index ee6715fa52..5ca2ca3852 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -495,7 +495,7 @@ static void batch_object_write(const char *obj_name, OBJECT_INFO_LOOKUP_REPLACE); if (ret < 0) { if (data->mode == S_IFGITLINK) - report_object_status(opt, oid_to_hex(&data->oid), &data->oid, "submodule"); + report_object_status(opt, NULL, &data->oid, "submodule"); else report_object_status(opt, obj_name, &data->oid, "missing"); return; base-commit: 4b71b294773cc4f7fe48ec3a70079aa8783f373d -- 2.51.0.491.g4b71b294773.dirty ``` ## Jeff King, 2025-10-14 00:11 Subject: Re: [PATCH] builtin/cat-file.c: simplify calling `report_object_status()` Message-ID: <20251014001128.GB1507@coredump.intra.peff.net> URL: https://gitlist.dev/e/20251014001128.GB1507%40coredump.intra.peff.net In-Reply-To: <9236fa6e654a59337266430a2a8807da21210cd4.1760392502.git.me@ttaylorr.com> ``` On Mon, Oct 13, 2025 at 05:56:01PM -0400, Taylor Blau wrote: > That commit does so by calling report_object_status() and passing in > "oid_to_hex(&data->oid)" for the "obj_name" parameter. This is > unnecessary, however, since report_object_status() will do the same > automatically if given a NULL "obj_name" argument. Yeah, looking at the code, this should obviously be a noop change, and I think it simplifies things a little. It is interesting that "oid" is not used in report_object_status() except for this fallback. Which kind of makes me wonder if we could ditch it completely, and just pass int oid_to_hex() unconditionally here. But it's hard to say if other code paths might end up with a NULL obj_name somehow (e.g., in an error path). ...poking at it... Ah, indeed. The patch below does fail one test in t5313 with a corrupted pack. So not worth pursuing that further simplification. Your patch looks good to me. :) -Peff -- >8 -- diff --git a/builtin/cat-file.c b/builtin/cat-file.c index ee6715fa52..19625b5a64 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -455,11 +455,9 @@ static void print_default_format(struct strbuf *scratch, struct expand_data *dat static void report_object_status(struct batch_options *opt, const char *obj_name, - const struct object_id *oid, const char *status) { - printf("%s %s%c", obj_name ? obj_name : oid_to_hex(oid), - status, opt->output_delim); + printf("%s %s%c", obj_name, status, opt->output_delim); fflush(stdout); } @@ -495,9 +493,9 @@ static void batch_object_write(const char *obj_name, OBJECT_INFO_LOOKUP_REPLACE); if (ret < 0) { if (data->mode == S_IFGITLINK) - report_object_status(opt, oid_to_hex(&data->oid), &data->oid, "submodule"); + report_object_status(opt, oid_to_hex(&data->oid), "submodule"); else - report_object_status(opt, obj_name, &data->oid, "missing"); + report_object_status(opt, obj_name, "missing"); return; } @@ -507,25 +505,22 @@ static void batch_object_write(const char *obj_name, case LOFC_BLOB_NONE: if (data->type == OBJ_BLOB) { if (!opt->all_objects) - report_object_status(opt, obj_name, - &data->oid, "excluded"); + report_object_status(opt, obj_name, "excluded"); return; } break; case LOFC_BLOB_LIMIT: if (data->type == OBJ_BLOB && data->size >= opt->objects_filter.blob_limit_value) { if (!opt->all_objects) - report_object_status(opt, obj_name, - &data->oid, "excluded"); + report_object_status(opt, obj_name, "excluded"); return; } break; case LOFC_OBJECT_TYPE: if (data->type != opt->objects_filter.object_type) { if (!opt->all_objects) - report_object_status(opt, obj_name, - &data->oid, "excluded"); + report_object_status(opt, obj_name, "excluded"); return; } break; @@ -581,10 +576,10 @@ static void batch_one_object(const char *obj_name, if (result != FOUND) { switch (result) { case MISSING_OBJECT: - report_object_status(opt, obj_name, &data->oid, "missing"); + report_object_status(opt, obj_name, "missing"); break; case SHORT_NAME_AMBIGUOUS: - report_object_status(opt, obj_name, &data->oid, "ambiguous"); + report_object_status(opt, obj_name, "ambiguous"); break; case DANGLING_SYMLINK: printf("dangling %"PRIuMAX"%c%s%c", ```