Christian Couder [off-list ref] writes:

On Mon, Dec 9, 2024 at 12:11 PM Karthik Nayak [off-list ref] wrote:

quoted

Unless the `REF_SKIP_REFNAME_VERIFICATION` flag is set for an update,
the refname of the update is verified for:

  - Ensuring it is not a pseudoref.
  - Checking the refname format.

These checks are also be needed in a following commit where the function

s/are also be needed/will also be needed/

Will amend.

quoted

to add reflog updates to the transaction is introduced. Extract the code
out into a new static function.

Signed-off-by: Karthik Nayak <redacted>
---
 refs.c | 43 ++++++++++++++++++++++++++++---------------
 1 file changed, 28 insertions(+), 15 deletions(-)

diff --git a/refs.c b/refs.c
index f003e51c6bf5229bfbce8ce61ffad7cdba0572e0..732c236a3fd0cf324cc172b48d3d54f6dbadf4a4 100644
--- a/refs.c
+++ b/refs.c

@@ -1196,6 +1196,29 @@ struct ref_update *ref_transaction_add_update(
        return update;
 }

+static int transaction_refname_verification(const char *refname,
+                                           const struct object_id *new_oid,
+                                           unsigned int flags,
+                                           struct strbuf *err)

We have a number of functions named 'xxx_valid()' or 'xxx_ok()' while
I couldn't find any 'yyy_verification()' function, so it might be
better to name it 'transaction_refname_valid()' or maybe
'transaction_refname_ok()'.

I think you're right, it helps to be consistent here. Will change to
`transaction_refname_valid()`.

Also I think it should probably return a bool so 1 if the refname is
valid and 0 otherwise, unless we have plans in the future to follow
different code paths depending on the different ways it is not valid.

That is a good idea.

quoted

+       ret = transaction_refname_verification(refname, new_oid, flags, err);
+       if (ret)
+               return ret;

Then the above could be just:

       if (!transaction_refname_valid(refname, new_oid, flags, err))
               return -1;

Yup, also will remove the need for the `ret` variable.