# [PATCH] use repo_get_oid_with_flags() 2 messages from 2025-09-10 to 2025-09-15. Participants: René Scharfe, Patrick Steinhardt. Thread: https://gitlist.dev/t/64122 ## René Scharfe, 2025-09-10 17:16 Subject: [PATCH] use repo_get_oid_with_flags() Message-ID: <906196ac-2fd7-4c07-9e8f-22d67b0b64f9@web.de> URL: https://gitlist.dev/e/906196ac-2fd7-4c07-9e8f-22d67b0b64f9%40web.de ``` get_oid_with_context() allows specifying flags and reports object details via a passed-in struct object_context. Some callers just want to specify flags, but don't need any details back. Convert them to repo_get_oid_with_flags(), which provides just that and frees them from dealing with the context structure. Signed-off-by: René Scharfe --- builtin/ls-tree.c | 7 ++----- builtin/rev-parse.c | 7 ++----- builtin/stash.c | 14 +++++--------- list-objects-filter.c | 9 +++------ object-name.c | 30 +++++------------------------- 5 files changed, 17 insertions(+), 50 deletions(-) diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c index 5d55731ca3..ec6940fc7c 100644 --- a/builtin/ls-tree.c +++ b/builtin/ls-tree.c @@ -373,7 +373,6 @@ int cmd_ls_tree(int argc, OPT_END() }; struct ls_tree_cmdmode_to_fmt *m2f = ls_tree_cmdmode_format; - struct object_context obj_context = {0}; int ret; repo_config(the_repository, git_default_config, NULL); @@ -405,9 +404,8 @@ int cmd_ls_tree(int argc, ls_tree_usage, ls_tree_options); if (argc < 1) usage_with_options(ls_tree_usage, ls_tree_options); - if (get_oid_with_context(the_repository, argv[0], - GET_OID_HASH_ANY, &oid, - &obj_context)) + if (repo_get_oid_with_flags(the_repository, argv[0], &oid, + GET_OID_HASH_ANY)) die("Not a valid object name %s", argv[0]); /* @@ -447,6 +445,5 @@ int cmd_ls_tree(int argc, ret = !!read_tree(the_repository, tree, &options.pathspec, fn, &options); clear_pathspec(&options.pathspec); - object_context_release(&obj_context); return ret; } diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c index 44ff1b8342..9da92b990d 100644 --- a/builtin/rev-parse.c +++ b/builtin/rev-parse.c @@ -708,7 +708,6 @@ int cmd_rev_parse(int argc, struct object_id oid; unsigned int flags = 0; const char *name = NULL; - struct object_context unused; struct strbuf buf = STRBUF_INIT; int seen_end_of_options = 0; enum format_type format = FORMAT_DEFAULT; @@ -1141,9 +1140,8 @@ int cmd_rev_parse(int argc, name++; type = REVERSED; } - if (!get_oid_with_context(the_repository, name, - flags, &oid, &unused)) { - object_context_release(&unused); + if (!repo_get_oid_with_flags(the_repository, name, &oid, + flags)) { if (output_algo) repo_oid_to_algop(the_repository, &oid, output_algo, &oid); @@ -1153,7 +1151,6 @@ int cmd_rev_parse(int argc, show_rev(type, &oid, name); continue; } - object_context_release(&unused); if (verify) die_no_single_rev(quiet); if (has_dashdash) diff --git a/builtin/stash.c b/builtin/stash.c index f5ddee5c7f..87acefb82f 100644 --- a/builtin/stash.c +++ b/builtin/stash.c @@ -1089,7 +1089,6 @@ static int store_stash(int argc, const char **argv, const char *prefix, int quiet = 0; const char *stash_msg = NULL; struct object_id obj; - struct object_context dummy = {0}; struct option options[] = { OPT__QUIET(&quiet, N_("be quiet")), OPT_STRING('m', "message", &stash_msg, "message", @@ -1109,9 +1108,8 @@ static int store_stash(int argc, const char **argv, const char *prefix, return -1; } - if (get_oid_with_context(the_repository, - argv[0], quiet ? GET_OID_QUIETLY : 0, &obj, - &dummy)) { + if (repo_get_oid_with_flags(the_repository, argv[0], &obj, + quiet ? GET_OID_QUIETLY : 0)) { if (!quiet) fprintf_ln(stderr, _("Cannot update %s with %s"), ref_stash, argv[0]); @@ -1122,7 +1120,6 @@ static int store_stash(int argc, const char **argv, const char *prefix, ret = do_store_stash(&obj, stash_msg, quiet); out: - object_context_release(&dummy); return ret; } @@ -2235,7 +2232,6 @@ static int do_export_stash(struct repository *r, const char **argv) { struct object_id base; - struct object_context unused; struct commit *prev; struct commit_list *items = NULL, **iter = &items, *cur; int res = 0; @@ -2269,9 +2265,9 @@ static int do_export_stash(struct repository *r, struct commit *stash; if (parse_stash_revision(&revision, argv[i], 1) || - get_oid_with_context(r, revision.buf, - GET_OID_QUIETLY | GET_OID_GENTLY, - &oid, &unused)) { + repo_get_oid_with_flags(r, revision.buf, &oid, + GET_OID_QUIETLY | + GET_OID_GENTLY)) { res = error(_("unable to find stash entry %s"), argv[i]); goto out; } diff --git a/list-objects-filter.c b/list-objects-filter.c index 7ecd4d9ef5..acd65ebb73 100644 --- a/list-objects-filter.c +++ b/list-objects-filter.c @@ -524,12 +524,11 @@ static void filter_sparse_oid__init( struct filter *filter) { struct filter_sparse_data *d = xcalloc(1, sizeof(*d)); - struct object_context oc; struct object_id sparse_oid; - if (get_oid_with_context(the_repository, - filter_options->sparse_oid_name, - GET_OID_BLOB, &sparse_oid, &oc)) + if (repo_get_oid_with_flags(the_repository, + filter_options->sparse_oid_name, + &sparse_oid, GET_OID_BLOB)) die(_("unable to access sparse blob in '%s'"), filter_options->sparse_oid_name); if (add_patterns_from_blob_to_list(&sparse_oid, "", 0, &d->pl) < 0) @@ -544,8 +543,6 @@ static void filter_sparse_oid__init( filter->filter_data = d; filter->filter_object_fn = filter_sparse; filter->free_fn = filter_sparse_free; - - object_context_release(&oc); } /* diff --git a/object-name.c b/object-name.c index 732056ff5e..52d87348d1 100644 --- a/object-name.c +++ b/object-name.c @@ -1858,55 +1858,35 @@ int repo_get_oid_committish(struct repository *r, const char *name, struct object_id *oid) { - struct object_context unused; - int ret = get_oid_with_context(r, name, GET_OID_COMMITTISH, - oid, &unused); - object_context_release(&unused); - return ret; + return repo_get_oid_with_flags(r, name, oid, GET_OID_COMMITTISH); } int repo_get_oid_treeish(struct repository *r, const char *name, struct object_id *oid) { - struct object_context unused; - int ret = get_oid_with_context(r, name, GET_OID_TREEISH, - oid, &unused); - object_context_release(&unused); - return ret; + return repo_get_oid_with_flags(r, name, oid, GET_OID_TREEISH); } int repo_get_oid_commit(struct repository *r, const char *name, struct object_id *oid) { - struct object_context unused; - int ret = get_oid_with_context(r, name, GET_OID_COMMIT, - oid, &unused); - object_context_release(&unused); - return ret; + return repo_get_oid_with_flags(r, name, oid, GET_OID_COMMIT); } int repo_get_oid_tree(struct repository *r, const char *name, struct object_id *oid) { - struct object_context unused; - int ret = get_oid_with_context(r, name, GET_OID_TREE, - oid, &unused); - object_context_release(&unused); - return ret; + return repo_get_oid_with_flags(r, name, oid, GET_OID_TREE); } int repo_get_oid_blob(struct repository *r, const char *name, struct object_id *oid) { - struct object_context unused; - int ret = get_oid_with_context(r, name, GET_OID_BLOB, - oid, &unused); - object_context_release(&unused); - return ret; + return repo_get_oid_with_flags(r, name, oid, GET_OID_BLOB); } /* Must be called only when object_name:filename doesn't exist. */ -- 2.51.0 ``` ## Patrick Steinhardt, 2025-09-15 06:16 Subject: Re: [PATCH] use repo_get_oid_with_flags() Message-ID: URL: https://gitlist.dev/e/aMevTM6YESMDdWPh%40pks.im In-Reply-To: <906196ac-2fd7-4c07-9e8f-22d67b0b64f9@web.de> ``` On Wed, Sep 10, 2025 at 07:16:30PM +0200, René Scharfe wrote: Nit: it would probably make sense to add something like a `treewide: ` prefix to the patch subject. > get_oid_with_context() allows specifying flags and reports object > details via a passed-in struct object_context. Some callers just want > to specify flags, but don't need any details back. Convert them to > repo_get_oid_with_flags(), which provides just that and frees them from > dealing with the context structure. Makes sense. `repo_get_oid_with_flags()` is a mere wrapper around `get_oid_with_context()` with an object context. So these two would be equivalent to one another. One thing that is a bit weird though is that `repo_get_oid_with_flags()` returns an `int` even though it directly returns the result from `get_oid_with_context()`, which returns an `enum get_oid_result`. That shouldn't impact correctness though, and it's not a new problem. > diff --git a/object-name.c b/object-name.c > index 732056ff5e..52d87348d1 100644 > --- a/object-name.c > +++ b/object-name.c > @@ -1858,55 +1858,35 @@ int repo_get_oid_committish(struct repository *r, > const char *name, > struct object_id *oid) > { > - struct object_context unused; > - int ret = get_oid_with_context(r, name, GET_OID_COMMITTISH, > - oid, &unused); > - object_context_release(&unused); > - return ret; > + return repo_get_oid_with_flags(r, name, oid, GET_OID_COMMITTISH); > } The same is true for all these functions. It would feel more sensible if those all returned `enum get_oid_result`. But as said, that's not a new problem and doesn't need fixing in your patch. Other than that all the conversions look correct to me, thanks! Patrick ```