Re: [PATCH v2 24/24] refs/files-backend: convert static functions to object_id

[Michael Haggerty]

On 10/09/2017 03:11 AM, brian m. carlson wrote:
> Convert several static functions to take pointers to struct object_id.
> Change the relevant parameters to write_packed_entry to be const, as we
> don't modify them.  Rename lock_ref_sha1_basic to lock_ref_oid_basic to
> reflect its new argument.
> 
> Signed-off-by: brian m. carlson 
> ---
>  refs/files-backend.c | 48 
>  1 file changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 7281f27f62..84d8e3da99 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -811,7 +811,7 @@ static struct ref_iterator *files_ref_iterator_begin(
>   * return a negative value.
>   */

The docstring for this function needs adjusting. It would also be worth
mentioning that `old_oid` is allowed to be NULL (which was true before
but wasn't mentioned).

> [...]

Michael

[PATCH v2 24/24] refs/files-backend: convert static functions to object_id

[brian m. carlson]

Convert several static functions to take pointers to struct object_id.
Change the relevant parameters to write_packed_entry to be const, as we
don't modify them.  Rename lock_ref_sha1_basic to lock_ref_oid_basic to
reflect its new argument.

Signed-off-by: brian m. carlson 
---
 refs/files-backend.c | 48 
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 7281f27f62..84d8e3da99 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -811,7 +811,7 @@ static struct ref_iterator *files_ref_iterator_begin(
  * return a negative value.
  */
 static int verify_lock(struct ref_store *ref_store, struct ref_lock *lock,
-  const unsigned char *old_sha1, int mustexist,
+  const struct object_id *old_oid, int mustexist,
   struct strbuf *err)
 {
assert(err);
@@ -819,7 +819,7 @@ static int verify_lock(struct ref_store *ref_store, struct 
ref_lock *lock,
if (refs_read_ref_full(ref_store, lock->ref_name,
   mustexist ? RESOLVE_REF_READING : 0,
   >old_oid, NULL)) {
-   if (old_sha1) {
+   if (old_oid) {
int save_errno = errno;
strbuf_addf(err, "can't verify ref '%s'", 
lock->ref_name);
errno = save_errno;
@@ -829,11 +829,11 @@ static int verify_lock(struct ref_store *ref_store, 
struct ref_lock *lock,
return 0;
}
}
-   if (old_sha1 && hashcmp(lock->old_oid.hash, old_sha1)) {
+   if (old_oid && oidcmp(>old_oid, old_oid)) {
strbuf_addf(err, "ref '%s' is at %s but expected %s",
lock->ref_name,
oid_to_hex(>old_oid),
-   sha1_to_hex(old_sha1));
+   oid_to_hex(old_oid));
errno = EBUSY;
return -1;
}
@@ -863,22 +863,22 @@ static int create_reflock(const char *path, void *cb)
  * Locks a ref returning the lock on success and NULL on failure.
  * On failure errno is set to something meaningful.
  */
-static struct ref_lock *lock_ref_sha1_basic(struct files_ref_store *refs,
-   const char *refname,
-   const unsigned char *old_sha1,
-   const struct string_list *extras,
-   const struct string_list *skip,
-   unsigned int flags, int *type,
-   struct strbuf *err)
+static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,
+  const char *refname,
+  const struct object_id *old_oid,
+  const struct string_list *extras,
+  const struct string_list *skip,
+  unsigned int flags, int *type,
+  struct strbuf *err)
 {
struct strbuf ref_file = STRBUF_INIT;
struct ref_lock *lock;
int last_errno = 0;
-   int mustexist = (old_sha1 && !is_null_sha1(old_sha1));
+   int mustexist = (old_oid && !is_null_oid(old_oid));
int resolve_flags = RESOLVE_REF_NO_RECURSE;
int resolved;
 
-   files_assert_main_repository(refs, "lock_ref_sha1_basic");
+   files_assert_main_repository(refs, "lock_ref_oid_basic");
assert(err);
 
lock = xcalloc(1, sizeof(struct ref_lock));
@@ -944,7 +944,7 @@ static struct ref_lock *lock_ref_sha1_basic(struct 
files_ref_store *refs,
goto error_return;
}
 
-   if (verify_lock(>base, lock, old_sha1, mustexist, err)) {
+   if (verify_lock(>base, lock, old_oid, mustexist, err)) {
last_errno = errno;
goto error_return;
}
@@ -1349,8 +1349,8 @@ static int files_rename_ref(struct ref_store *ref_store,
 
logmoved = log;
 
-   lock = lock_ref_sha1_basic(refs, newrefname, NULL, NULL, NULL,
-  REF_NODEREF, NULL, );
+   lock = lock_ref_oid_basic(refs, newrefname, NULL, NULL, NULL,
+ REF_NODEREF, NULL, );
if (!lock) {
error("unable to rename '%s' to '%s': %s", oldrefname, 
newrefname, err.buf);
strbuf_release();
@@ -1369,8 +1369,8 @@ static int files_rename_ref(struct ref_store *ref_store,
goto out;
 
  rollback:
-   lock = lock_ref_sha1_basic(refs, oldrefname, NULL, NULL, NULL,
-  REF_NODEREF, NULL, );
+   lock = lock_ref_oid_basic(refs, oldrefname, NULL, NULL, NULL,
+