git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 05/20] refs_verify_refname_available(): use function in more places

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Mar 31, 2017, 14:11 UTC
Message-ID
<80808305cd86f4fb3b8b06cd78ba8d0a9cbf11a5.1490966385.git.mhagger@alum.mit.edu>
In-Reply-To
<cover.1490966385.git.mhagger@alum.mit.edu>

Change `lock_raw_ref()` and `lock_ref_sha1_basic()` to use `refs_verify_refname_available()` instead of `verify_refname_available_dir()`. This means that those callsites now check for conflicts with all references rather than just packed refs, but the performance cost shouldn't be significant (and will be regained later).

These were the last callers of `verify_refname_available_dir()`, so also delete that (very complicated) function.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs/files-backend.c | 171 ++++-----------------------------------------------
 1 file changed, 11 insertions(+), 160 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 447f2767f3..cad56efb04 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -738,152 +738,6 @@ static struct ref_iterator *cache_ref_iterator_begin(struct ref_dir *dir)
 	return ref_iterator;
 }
 
-struct nonmatching_ref_data {
-	const struct string_list *skip;
-	const char *conflicting_refname;
-};
-
-static int nonmatching_ref_fn(struct ref_entry *entry, void *vdata)
-{
-	struct nonmatching_ref_data *data = vdata;
-
-	if (data->skip && string_list_has_string(data->skip, entry->name))
-		return 0;
-
-	data->conflicting_refname = entry->name;
-	return 1;
-}
-
-/*
- * Return 0 if a reference named refname could be created without
- * conflicting with the name of an existing reference in dir.
- * See verify_refname_available for more information.
- */
-static int verify_refname_available_dir(const char *refname,
-					const struct string_list *extras,
-					const struct string_list *skip,
-					struct ref_dir *dir,
-					struct strbuf *err)
-{
-	const char *slash;
-	const char *extra_refname;
-	int pos;
-	struct strbuf dirname = STRBUF_INIT;
-	int ret = -1;
-
-	/*
-	 * For the sake of comments in this function, suppose that
-	 * refname is "refs/foo/bar".
-	 */
-
-	assert(err);
-
-	strbuf_grow(&dirname, strlen(refname) + 1);
-	for (slash = strchr(refname, '/'); slash; slash = strchr(slash + 1, '/')) {
-		/* Expand dirname to the new prefix, not including the trailing slash: */
-		strbuf_add(&dirname, refname + dirname.len, slash - refname - dirname.len);
-
-		/*
-		 * We are still at a leading dir of the refname (e.g.,
-		 * "refs/foo"; if there is a reference with that name,
-		 * it is a conflict, *unless* it is in skip.
-		 */
-		if (dir) {
-			pos = search_ref_dir(dir, dirname.buf, dirname.len);
-			if (pos >= 0 &&
-			    (!skip || !string_list_has_string(skip, dirname.buf))) {
-				/*
-				 * We found a reference whose name is
-				 * a proper prefix of refname; e.g.,
-				 * "refs/foo", and is not in skip.
-				 */
-				strbuf_addf(err, "'%s' exists; cannot create '%s'",
-					    dirname.buf, refname);
-				goto cleanup;
-			}
-		}
-
-		if (extras && string_list_has_string(extras, dirname.buf) &&
-		    (!skip || !string_list_has_string(skip, dirname.buf))) {
-			strbuf_addf(err, "cannot process '%s' and '%s' at the same time",
-				    refname, dirname.buf);
-			goto cleanup;
-		}
-
-		/*
-		 * Otherwise, we can try to continue our search with
-		 * the next component. So try to look up the
-		 * directory, e.g., "refs/foo/". If we come up empty,
-		 * we know there is nothing under this whole prefix,
-		 * but even in that case we still have to continue the
-		 * search for conflicts with extras.
-		 */
-		strbuf_addch(&dirname, '/');
-		if (dir) {
-			pos = search_ref_dir(dir, dirname.buf, dirname.len);
-			if (pos < 0) {
-				/*
-				 * There was no directory "refs/foo/",
-				 * so there is nothing under this
-				 * whole prefix. So there is no need
-				 * to continue looking for conflicting
-				 * references. But we need to continue
-				 * looking for conflicting extras.
-				 */
-				dir = NULL;
-			} else {
-				dir = get_ref_dir(dir->entries[pos]);
-			}
-		}
-	}
-
-	/*
-	 * We are at the leaf of our refname (e.g., "refs/foo/bar").
-	 * There is no point in searching for a reference with that
-	 * name, because a refname isn't considered to conflict with
-	 * itself. But we still need to check for references whose
-	 * names are in the "refs/foo/bar/" namespace, because they
-	 * *do* conflict.
-	 */
-	strbuf_addstr(&dirname, refname + dirname.len);
-	strbuf_addch(&dirname, '/');
-
-	if (dir) {
-		pos = search_ref_dir(dir, dirname.buf, dirname.len);
-
-		if (pos >= 0) {
-			/*
-			 * We found a directory named "$refname/"
-			 * (e.g., "refs/foo/bar/"). It is a problem
-			 * iff it contains any ref that is not in
-			 * "skip".
-			 */
-			struct nonmatching_ref_data data;
-
-			data.skip = skip;
-			data.conflicting_refname = NULL;
-			dir = get_ref_dir(dir->entries[pos]);
-			sort_ref_dir(dir);
-			if (do_for_each_entry_in_dir(dir, 0, nonmatching_ref_fn, &data)) {
-				strbuf_addf(err, "'%s' exists; cannot create '%s'",
-					    data.conflicting_refname, refname);
-				goto cleanup;
-			}
-		}
-	}
-
-	extra_refname = find_descendant_ref(dirname.buf, extras, skip);
-	if (extra_refname)
-		strbuf_addf(err, "cannot process '%s' and '%s' at the same time",
-			    refname, extra_refname);
-	else
-		ret = 0;
-
-cleanup:
-	strbuf_release(&dirname);
-	return ret;
-}
-
 struct packed_ref_cache {
 	struct ref_entry *root;
 
@@ -1563,7 +1417,7 @@ static void unlock_ref(struct ref_lock *lock)
  *
  * If the reference doesn't already exist, verify that refname doesn't
  * have a D/F conflict with any existing references. extras and skip
- * are passed to verify_refname_available_dir() for this check.
+ * are passed to refs_verify_refname_available() for this check.
  *
  * If mustexist is not set and the reference is not found or is
  * broken, lock the reference anyway but clear sha1.
@@ -1578,7 +1432,7 @@ static void unlock_ref(struct ref_lock *lock)
  *
  * but it includes a lot more code to
  * - Deal with possible races with other processes
- * - Avoid calling verify_refname_available_dir() when it can be
+ * - Avoid calling refs_verify_refname_available() when it can be
  *   avoided, namely if we were successfully able to read the ref
  * - Generate informative error messages in the case of failure
  */
@@ -1635,7 +1489,8 @@ static int lock_raw_ref(struct files_ref_store *refs,
 			} else {
 				/*
 				 * The error message set by
-				 * verify_refname_available_dir() is OK.
+				 * refs_verify_refname_available() is
+				 * OK.
 				 */
 				ret = TRANSACTION_NAME_CONFLICT;
 			}
@@ -1759,16 +1614,13 @@ static int lock_raw_ref(struct files_ref_store *refs,
 
 		/*
 		 * If the ref did not exist and we are creating it,
-		 * make sure there is no existing packed ref whose
-		 * name begins with our refname, nor a packed ref
-		 * whose name is a proper prefix of our refname.
+		 * make sure there is no existing ref that conflicts
+		 * with refname:
 		 */
-		if (verify_refname_available_dir(
-				    refname, extras, skip,
-				    get_packed_refs(refs),
-				    err)) {
+		if (refs_verify_refname_available(
+				    &refs->base, refname,
+				    extras, skip, err))
 			goto error_return;
-		}
 	}
 
 	ret = 0;
@@ -2125,9 +1977,8 @@ static struct ref_lock *lock_ref_sha1_basic(struct files_ref_store *refs,
 	 * our refname.
 	 */
 	if (is_null_oid(&lock->old_oid) &&
-	    verify_refname_available_dir(refname, extras, skip,
-					 get_packed_refs(refs),
-					 err)) {
+	    refs_verify_refname_available(&refs->base, refname,
+					  extras, skip, err)) {
 		last_errno = ENOTDIR;
 		goto error_return;
 	}

-- 
2.11.0
Previous: Michael HaggertyNext: Michael Haggerty
Message 14 of 42 in “Separate `ref_cache` into a separate module”
  1. 00/20 Separate `ref_cache` into a separate moduleMichael Haggerty, Mar 31, 2017
  2. 01/20 get_ref_dir(): don't call read_loose_refs() for "refs/bisect"Michael Haggerty, Mar 31, 2017
  3. Stefan BellerMar 31, 2017
  4. 02/20 refs_read_raw_ref(): new functionMichael Haggerty, Mar 31, 2017
  5. 04/20 refs_verify_refname_available(): implement once for all backendsMichael Haggerty, Mar 31, 2017
  6. Duy NguyenApr 7, 2017
  7. Michael HaggertyApr 16, 2017
  8. 06/20 ref-cache: rename `add_ref()` to `add_ref_entry()`Michael Haggerty, Mar 31, 2017
  9. 07/20 ref-cache: rename `find_ref()` to `find_ref_entry()`Michael Haggerty, Mar 31, 2017
  10. 08/20 ref-cache: rename `remove_entry()` to `remove_entry_from_dir()`Michael Haggerty, Mar 31, 2017
  11. 03/20 refs_ref_iterator_begin(): new functionMichael Haggerty, Mar 31, 2017
  12. Duy NguyenApr 7, 2017
  13. Michael HaggertyApr 16, 2017
  14. 05/20 refs_verify_refname_available(): use function in more placesMichael Haggerty, Mar 31, 2017
  15. 12/20 ref-cache: use a callback function to fill the cacheMichael Haggerty, Mar 31, 2017
  16. 09/20 refs: split `ref_cache` code into separate filesMichael Haggerty, Mar 31, 2017
  17. 15/20 get_loose_ref_dir(): function renamed from get_loose_refs()Michael Haggerty, Mar 31, 2017
  18. 16/20 get_loose_ref_cache(): new functionMichael Haggerty, Mar 31, 2017
  19. 19/20 files_pack_refs(): use reference iterationMichael Haggerty, Mar 31, 2017
  20. Duy NguyenApr 7, 2017
  21. Michael HaggertyApr 16, 2017
  22. 17/20 cache_ref_iterator_begin(): make function smarterMichael Haggerty, Mar 31, 2017
  23. 18/20 commit_packed_refs(): use reference iterationMichael Haggerty, Mar 31, 2017
  24. 13/20 refs: handle "refs/bisect/" in `loose_fill_ref_dir()`Michael Haggerty, Mar 31, 2017
  25. 20/20 do_for_each_entry_in_dir(): delete functionMichael Haggerty, Mar 31, 2017
  26. 10/20 ref-cache: introduce a new type, ref_cacheMichael Haggerty, Mar 31, 2017
  27. Duy NguyenApr 7, 2017
  28. Michael HaggertyApr 16, 2017
  29. 14/20 do_for_each_entry_in_dir(): eliminate `offset` argumentMichael Haggerty, Mar 31, 2017
  30. 11/20 refs: record the ref_store in ref_cache, not ref_dirMichael Haggerty, Mar 31, 2017
  31. Duy NguyenApr 7, 2017
  32. Michael HaggertyApr 16, 2017
  33. Junio C HamanoMar 31, 2017
  34. Michael HaggertyApr 1, 2017
  35. Duy NguyenApr 5, 2017
  36. Duy NguyenApr 7, 2017
  37. Michael HaggertyApr 16, 2017
  38. Ramsay JonesApr 1, 2017
  39. Junio C HamanoApr 2, 2017
  40. Ramsay JonesApr 2, 2017
  41. Junio C HamanoApr 2, 2017
  42. Jeff KingApr 1, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.