From: Karthik Nayak Date: Sun, 21 Apr 2024 19:00:11 GMT Subject: Re: [PATCH v2 6/7] update-ref: add support for symref-update Message-ID: In-Reply-To: Patrick Steinhardt writes: [snip] >> + /* >> + * Since the user can also send in an old-oid, we try to parse >> + * it as such too. >> + */ >> + if (old_ref && read_ref(old_ref, NULL)) { >> + if (!repo_get_oid(the_repository, old_ref, &old_oid)) { >> + old_ref = NULL; >> + have_old = 1; >> + } else >> + die("symref-update %s: invalid or ", refname); >> + } > > So we first try to parse it as a ref, and then as an object ID? Wouldn't > it preferable to try it the other way round and first check whether it > is a valid object ID? That would likely be cheaper, even though it may > be premature optimization. > > Patrick I think the issue is `repo_get_oid` would also parse a refname to an OID. Whereas we want to first check and keep refnames and only if it isn't a refname, we'd want to parse it as an OID. Also, why do you say it is cheaper?