[RFC PATCH 1/2] refs: pass struct repository *r through to write_ref_to_lockfile()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



In refs/files-backend.c, write_ref_to_lockfile() uses the_repository to
validate the oid of the ref being written. Thus, any function that
includes write_ref_to_lockfile() in its call chain has an implicit
dependence on the odb of the_repository. This is undesirable in the case
where we are using oids that are not in the_repository, e.g. when we are
updating refs in a submodule.

Let's fix this by passing struct repository * as a parameter through to
write_ref_to_lockfile().

To avoid breaking existing call sites, the existing function
signatures in refs.h are preserved. struct repository * is only passed
to new functions; these new functions have names prefixed with repo_.

Signed-off-by: Glen Choo <chooglen@xxxxxxxxxx>
---
 refs.c                    | 21 +++++++++++----------
 refs.h                    | 18 ++++++++++++++----
 refs/debug.c              | 14 ++++++++------
 refs/files-backend.c      | 30 +++++++++++++++---------------
 refs/packed-backend.c     |  7 ++++---
 refs/refs-internal.h      |  7 ++++---
 t/helper/test-ref-store.c |  2 +-
 7 files changed, 57 insertions(+), 42 deletions(-)

diff --git a/refs.c b/refs.c
index 8b9f7c3a80..c217512cd3 100644
--- a/refs.c
+++ b/refs.c
@@ -2105,7 +2105,7 @@ static int run_transaction_hook(struct ref_transaction *transaction,
 	return ret;
 }
 
-int ref_transaction_prepare(struct ref_transaction *transaction,
+int repo_ref_transaction_prepare(struct repository *r, struct ref_transaction *transaction,
 			    struct strbuf *err)
 {
 	struct ref_store *refs = transaction->ref_store;
@@ -2132,7 +2132,7 @@ int ref_transaction_prepare(struct ref_transaction *transaction,
 		return -1;
 	}
 
-	ret = refs->be->transaction_prepare(refs, transaction, err);
+	ret = refs->be->transaction_prepare(r, refs, transaction, err);
 	if (ret)
 		return ret;
 
@@ -2172,7 +2172,8 @@ int ref_transaction_abort(struct ref_transaction *transaction,
 	return ret;
 }
 
-int ref_transaction_commit(struct ref_transaction *transaction,
+int repo_ref_transaction_commit(struct repository *r,
+			   struct ref_transaction *transaction,
 			   struct strbuf *err)
 {
 	struct ref_store *refs = transaction->ref_store;
@@ -2181,7 +2182,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,
 	switch (transaction->state) {
 	case REF_TRANSACTION_OPEN:
 		/* Need to prepare first. */
-		ret = ref_transaction_prepare(transaction, err);
+		ret = repo_ref_transaction_prepare(r, transaction, err);
 		if (ret)
 			return ret;
 		break;
@@ -2421,36 +2422,36 @@ int delete_refs(const char *msg, struct string_list *refnames,
 	return refs_delete_refs(get_main_ref_store(the_repository), msg, refnames, flags);
 }
 
-int refs_rename_ref(struct ref_store *refs, const char *oldref,
+int refs_rename_ref(struct repository *r, struct ref_store *refs, const char *oldref,
 		    const char *newref, const char *logmsg)
 {
 	char *msg;
 	int retval;
 
 	msg = normalize_reflog_message(logmsg);
-	retval = refs->be->rename_ref(refs, oldref, newref, msg);
+	retval = refs->be->rename_ref(r, refs, oldref, newref, msg);
 	free(msg);
 	return retval;
 }
 
 int rename_ref(const char *oldref, const char *newref, const char *logmsg)
 {
-	return refs_rename_ref(get_main_ref_store(the_repository), oldref, newref, logmsg);
+	return refs_rename_ref(the_repository, get_main_ref_store(the_repository), oldref, newref, logmsg);
 }
 
-int refs_copy_existing_ref(struct ref_store *refs, const char *oldref,
+int refs_copy_existing_ref(struct repository *r, struct ref_store *refs, const char *oldref,
 		    const char *newref, const char *logmsg)
 {
 	char *msg;
 	int retval;
 
 	msg = normalize_reflog_message(logmsg);
-	retval = refs->be->copy_ref(refs, oldref, newref, msg);
+	retval = refs->be->copy_ref(r, refs, oldref, newref, msg);
 	free(msg);
 	return retval;
 }
 
 int copy_existing_ref(const char *oldref, const char *newref, const char *logmsg)
 {
-	return refs_copy_existing_ref(get_main_ref_store(the_repository), oldref, newref, logmsg);
+	return refs_copy_existing_ref(the_repository, get_main_ref_store(the_repository), oldref, newref, logmsg);
 }
diff --git a/refs.h b/refs.h
index 48970dfc7e..157d14d7a3 100644
--- a/refs.h
+++ b/refs.h
@@ -524,13 +524,13 @@ char *refs_shorten_unambiguous_ref(struct ref_store *refs,
 char *shorten_unambiguous_ref(const char *refname, int strict);
 
 /** rename ref, return 0 on success **/
-int refs_rename_ref(struct ref_store *refs, const char *oldref,
+int refs_rename_ref(struct repository *r, struct ref_store *refs, const char *oldref,
 		    const char *newref, const char *logmsg);
 int rename_ref(const char *oldref, const char *newref,
 			const char *logmsg);
 
 /** copy ref, return 0 on success **/
-int refs_copy_existing_ref(struct ref_store *refs, const char *oldref,
+int refs_copy_existing_ref(struct repository *r, struct ref_store *refs, const char *oldref,
 		    const char *newref, const char *logmsg);
 int copy_existing_ref(const char *oldref, const char *newref,
 			const char *logmsg);
@@ -706,8 +706,13 @@ int ref_transaction_verify(struct ref_transaction *transaction,
  * Callers who don't need such fine-grained control over committing
  * reference transactions should just call `ref_transaction_commit()`.
  */
-int ref_transaction_prepare(struct ref_transaction *transaction,
+int repo_ref_transaction_prepare(struct repository *r, struct ref_transaction *transaction,
 			    struct strbuf *err);
+static inline int ref_transaction_prepare(struct ref_transaction *transaction,
+					  struct strbuf *err)
+{
+	return repo_ref_transaction_prepare(the_repository, transaction, err);
+}
 
 /*
  * Commit all of the changes that have been queued in transaction, as
@@ -716,8 +721,13 @@ int ref_transaction_prepare(struct ref_transaction *transaction,
  * transaction, write an error message to `err`, and return one of the
  * `TRANSACTION_*` constants
  */
-int ref_transaction_commit(struct ref_transaction *transaction,
+int repo_ref_transaction_commit(struct repository *r, struct ref_transaction *transaction,
 			   struct strbuf *err);
+static inline int ref_transaction_commit(struct ref_transaction *transaction,
+					 struct strbuf *err)
+{
+	return repo_ref_transaction_commit(the_repository, transaction, err);
+}
 
 /*
  * Abort `transaction`, which has been begun and possibly prepared,
diff --git a/refs/debug.c b/refs/debug.c
index 1a7a9e11cf..224ee76563 100644
--- a/refs/debug.c
+++ b/refs/debug.c
@@ -38,14 +38,15 @@ static int debug_init_db(struct ref_store *refs, struct strbuf *err)
 	return res;
 }
 
-static int debug_transaction_prepare(struct ref_store *refs,
+static int debug_transaction_prepare(struct repository *r,
+				     struct ref_store *refs,
 				     struct ref_transaction *transaction,
 				     struct strbuf *err)
 {
 	struct debug_ref_store *drefs = (struct debug_ref_store *)refs;
 	int res;
 	transaction->ref_store = drefs->refs;
-	res = drefs->refs->be->transaction_prepare(drefs->refs, transaction,
+	res = drefs->refs->be->transaction_prepare(r, drefs->refs, transaction,
 						   err);
 	trace_printf_key(&trace_refs, "transaction_prepare: %d\n", res);
 	return res;
@@ -153,23 +154,24 @@ static int debug_delete_refs(struct ref_store *ref_store, const char *msg,
 	return res;
 }
 
-static int debug_rename_ref(struct ref_store *ref_store, const char *oldref,
+static int debug_rename_ref(struct repository *r,
+			    struct ref_store *ref_store, const char *oldref,
 			    const char *newref, const char *logmsg)
 {
 	struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
-	int res = drefs->refs->be->rename_ref(drefs->refs, oldref, newref,
+	int res = drefs->refs->be->rename_ref(r, drefs->refs, oldref, newref,
 					      logmsg);
 	trace_printf_key(&trace_refs, "rename_ref: %s -> %s \"%s\": %d\n", oldref, newref,
 		logmsg, res);
 	return res;
 }
 
-static int debug_copy_ref(struct ref_store *ref_store, const char *oldref,
+static int debug_copy_ref(struct repository *r, struct ref_store *ref_store, const char *oldref,
 			  const char *newref, const char *logmsg)
 {
 	struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
 	int res =
-		drefs->refs->be->copy_ref(drefs->refs, oldref, newref, logmsg);
+		drefs->refs->be->copy_ref(r, drefs->refs, oldref, newref, logmsg);
 	trace_printf_key(&trace_refs, "copy_ref: %s -> %s \"%s\": %d\n", oldref, newref,
 		logmsg, res);
 	return res;
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 74c0385873..2bd6115484 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1308,14 +1308,14 @@ static int rename_tmp_log(struct files_ref_store *refs, const char *newrefname)
 	return ret;
 }
 
-static int write_ref_to_lockfile(struct ref_lock *lock,
+static int write_ref_to_lockfile(struct repository *r, struct ref_lock *lock,
 				 const struct object_id *oid, struct strbuf *err);
 static int commit_ref_update(struct files_ref_store *refs,
 			     struct ref_lock *lock,
 			     const struct object_id *oid, const char *logmsg,
 			     struct strbuf *err);
 
-static int files_copy_or_rename_ref(struct ref_store *ref_store,
+static int files_copy_or_rename_ref(struct repository *r, struct ref_store *ref_store,
 			    const char *oldrefname, const char *newrefname,
 			    const char *logmsg, int copy)
 {
@@ -1427,7 +1427,7 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,
 	}
 	oidcpy(&lock->old_oid, &orig_oid);
 
-	if (write_ref_to_lockfile(lock, &orig_oid, &err) ||
+	if (write_ref_to_lockfile(r, lock, &orig_oid, &err) ||
 	    commit_ref_update(refs, lock, &orig_oid, logmsg, &err)) {
 		error("unable to write current sha1 into %s: %s", newrefname, err.buf);
 		strbuf_release(&err);
@@ -1448,7 +1448,7 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,
 
 	flag = log_all_ref_updates;
 	log_all_ref_updates = LOG_REFS_NONE;
-	if (write_ref_to_lockfile(lock, &orig_oid, &err) ||
+	if (write_ref_to_lockfile(r, lock, &orig_oid, &err) ||
 	    commit_ref_update(refs, lock, &orig_oid, NULL, &err)) {
 		error("unable to write current sha1 into %s: %s", oldrefname, err.buf);
 		strbuf_release(&err);
@@ -1472,19 +1472,19 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,
 	return ret;
 }
 
-static int files_rename_ref(struct ref_store *ref_store,
+static int files_rename_ref(struct repository *r, struct ref_store *ref_store,
 			    const char *oldrefname, const char *newrefname,
 			    const char *logmsg)
 {
-	return files_copy_or_rename_ref(ref_store, oldrefname,
+	return files_copy_or_rename_ref(r, ref_store, oldrefname,
 				 newrefname, logmsg, 0);
 }
 
-static int files_copy_ref(struct ref_store *ref_store,
+static int files_copy_ref(struct repository *r, struct ref_store *ref_store,
 			    const char *oldrefname, const char *newrefname,
 			    const char *logmsg)
 {
-	return files_copy_or_rename_ref(ref_store, oldrefname,
+	return files_copy_or_rename_ref(r, ref_store, oldrefname,
 				 newrefname, logmsg, 1);
 }
 
@@ -1683,14 +1683,14 @@ static int files_log_ref_write(struct files_ref_store *refs,
  * Write oid into the open lockfile, then close the lockfile. On
  * errors, rollback the lockfile, fill in *err and return -1.
  */
-static int write_ref_to_lockfile(struct ref_lock *lock,
+static int write_ref_to_lockfile(struct repository *r, struct ref_lock *lock,
 				 const struct object_id *oid, struct strbuf *err)
 {
 	static char term = '\n';
 	struct object *o;
 	int fd;
 
-	o = parse_object(the_repository, oid);
+	o = parse_object(r, oid);
 	if (!o) {
 		strbuf_addf(err,
 			    "trying to write ref '%s' with nonexistent object %s",
@@ -2390,7 +2390,7 @@ static int check_old_oid(struct ref_update *update, struct object_id *oid,
  * - If it is an update of head_ref, add a corresponding REF_LOG_ONLY
  *   update of HEAD.
  */
-static int lock_ref_for_update(struct files_ref_store *refs,
+static int lock_ref_for_update(struct repository *r, struct files_ref_store *refs,
 			       struct ref_update *update,
 			       struct ref_transaction *transaction,
 			       const char *head_ref,
@@ -2496,7 +2496,7 @@ static int lock_ref_for_update(struct files_ref_store *refs,
 			 * The reference already has the desired
 			 * value, so we don't need to write it.
 			 */
-		} else if (write_ref_to_lockfile(lock, &update->new_oid,
+		} else if (write_ref_to_lockfile(r, lock, &update->new_oid,
 						 err)) {
 			char *write_err = strbuf_detach(err, NULL);
 
@@ -2577,7 +2577,7 @@ static void files_transaction_cleanup(struct files_ref_store *refs,
 	transaction->state = REF_TRANSACTION_CLOSED;
 }
 
-static int files_transaction_prepare(struct ref_store *ref_store,
+static int files_transaction_prepare(struct repository *r, struct ref_store *ref_store,
 				     struct ref_transaction *transaction,
 				     struct strbuf *err)
 {
@@ -2667,7 +2667,7 @@ static int files_transaction_prepare(struct ref_store *ref_store,
 	for (i = 0; i < transaction->nr; i++) {
 		struct ref_update *update = transaction->updates[i];
 
-		ret = lock_ref_for_update(refs, update, transaction,
+		ret = lock_ref_for_update(r, refs, update, transaction,
 					  head_ref, &affected_refnames, err);
 		if (ret)
 			goto cleanup;
@@ -2708,7 +2708,7 @@ static int files_transaction_prepare(struct ref_store *ref_store,
 
 		if (is_packed_transaction_needed(refs->packed_ref_store,
 						 packed_transaction)) {
-			ret = ref_transaction_prepare(packed_transaction, err);
+			ret = repo_ref_transaction_prepare(r, packed_transaction, err);
 			/*
 			 * A failure during the prepare step will abort
 			 * itself, but not free. Do that now, and disconnect
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index f8aa97d799..e6aa1841ba 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1403,7 +1403,8 @@ static void packed_transaction_cleanup(struct packed_ref_store *refs,
 	transaction->state = REF_TRANSACTION_CLOSED;
 }
 
-static int packed_transaction_prepare(struct ref_store *ref_store,
+static int packed_transaction_prepare(struct repository *r,
+				      struct ref_store *ref_store,
 				      struct ref_transaction *transaction,
 				      struct strbuf *err)
 {
@@ -1577,14 +1578,14 @@ static int packed_create_symref(struct ref_store *ref_store,
 	BUG("packed reference store does not support symrefs");
 }
 
-static int packed_rename_ref(struct ref_store *ref_store,
+static int packed_rename_ref(struct repository *r, struct ref_store *ref_store,
 			    const char *oldrefname, const char *newrefname,
 			    const char *logmsg)
 {
 	BUG("packed reference store does not support renaming references");
 }
 
-static int packed_copy_ref(struct ref_store *ref_store,
+static int packed_copy_ref(struct repository *r, struct ref_store *ref_store,
 			   const char *oldrefname, const char *newrefname,
 			   const char *logmsg)
 {
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 3155708345..1b7a68db8c 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -530,7 +530,8 @@ typedef struct ref_store *ref_store_init_fn(const char *gitdir,
 
 typedef int ref_init_db_fn(struct ref_store *refs, struct strbuf *err);
 
-typedef int ref_transaction_prepare_fn(struct ref_store *refs,
+typedef int ref_transaction_prepare_fn(struct repository *r,
+				       struct ref_store *refs,
 				       struct ref_transaction *transaction,
 				       struct strbuf *err);
 
@@ -553,10 +554,10 @@ typedef int create_symref_fn(struct ref_store *ref_store,
 			     const char *logmsg);
 typedef int delete_refs_fn(struct ref_store *ref_store, const char *msg,
 			   struct string_list *refnames, unsigned int flags);
-typedef int rename_ref_fn(struct ref_store *ref_store,
+typedef int rename_ref_fn(struct repository *r, struct ref_store *ref_store,
 			  const char *oldref, const char *newref,
 			  const char *logmsg);
-typedef int copy_ref_fn(struct ref_store *ref_store,
+typedef int copy_ref_fn(struct repository *r, struct ref_store *ref_store,
 			  const char *oldref, const char *newref,
 			  const char *logmsg);
 
diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index b314b81a45..ad47488e5b 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -99,7 +99,7 @@ static int cmd_rename_ref(struct ref_store *refs, const char **argv)
 	const char *newref = notnull(*argv++, "newref");
 	const char *logmsg = *argv++;
 
-	return refs_rename_ref(refs, oldref, newref, logmsg);
+	return refs_rename_ref(the_repository, refs, oldref, newref, logmsg);
 }
 
 static int each_ref(const char *refname, const struct object_id *oid,
-- 
2.33.0.464.g1972c5931b-goog




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux