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

[PATCH 3/4] lock_ref_sha1_basic: simplify error code path

From
Jeff King <peff@peff.net>
Date
Nov 19, 2014, 01:37 UTC
Message-ID
<20141119013739.GC2135@peff.net>
In-Reply-To
<20141119013532.GA861@peff.net>

For most errors, we jump to a goto label that unlocks the ref and returns NULL. However, in none of these error paths would we ever have actually locked the ref. By the time we actually take the lock, we follow a different path that does not ever hit this goto (we rely on verify_lock to unlock if it finds an error).

We can just drop the goto completely and return NULL as appropriate.

Signed-off-by: Jeff King <peff@peff.net>
---
 refs.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/refs.c b/refs.c
index 169a46d..fafae7e 100644
--- a/refs.c
+++ b/refs.c
@@ -2260,7 +2260,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
 		ref_file = git_path("%s", orig_refname);
 		if (remove_empty_directories(ref_file)) {
 			error("there are still refs under '%s'", orig_refname);
-			goto error_return;
+			return NULL;
 		}
 		refname = resolve_ref_unsafe(orig_refname, resolve_flags,
 					     lock->old_sha1, &type);
@@ -2270,7 +2270,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
 	if (!refname) {
 		error("unable to resolve reference %s: %s",
 			orig_refname, strerror(errno));
-		goto error_return;
+		return NULL;
 	}
 	missing = is_null_sha1(lock->old_sha1);
 	/* When the ref did not exist and we are creating it,
@@ -2281,7 +2281,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
 	if (missing &&
 	     !is_refname_available(refname, skip, get_packed_refs(&ref_cache))) {
 		errno = ENOTDIR;
-		goto error_return;
+		return NULL;
 	}
 
 	lock->lk = xcalloc(1, sizeof(struct lock_file));
@@ -2309,7 +2309,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
 		/* fall through */
 	default:
 		error("unable to create directory for %s", ref_file);
-		goto error_return;
+		return NULL;
 	}
 
 	lock->lock_fd = hold_lock_file_for_update(lock->lk, ref_file, lflags);
@@ -2325,10 +2325,6 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
 			unable_to_lock_die(ref_file, errno);
 	}
 	return old_sha1 ? verify_lock(lock, old_sha1, mustexist) : lock;
-
- error_return:
-	unlock_ref(lock);
-	return NULL;
 }
 
 struct ref_lock *lock_any_ref_for_update(const char *refname,
-- 
2.1.2.596.g7379948
Previous: Michael HaggertyNext: Jonathan Nieder
Message 14 of 24 in “refs.c: handle locking failure during transaction better”
  1. refs.c: handle locking failure during transaction betterStefan Beller, Nov 18, 2014
  2. Stefan BellerNov 18, 2014
  3. refs.c: handle locking failure during transaction betterStefan Beller, Nov 19, 2014
  4. 0/4 error cleanups in lock_ref_sha1_basicJeff King, Nov 19, 2014
  5. 1/4 error: save and restore errnoJeff King, Nov 19, 2014
  6. Stefan BellerNov 19, 2014
  7. Jonathan NiederNov 19, 2014
  8. Jeff KingNov 19, 2014
  9. Junio C HamanoNov 19, 2014
  10. Jeff KingNov 19, 2014
  11. 2/4 lock_ref_sha1_basic: simplify errno handlingJeff King, Nov 19, 2014
  12. Jonathan NiederNov 19, 2014
  13. Michael HaggertyNov 21, 2014
  14. 3/4 lock_ref_sha1_basic: simplify error code pathJeff King, Nov 19, 2014
  15. Jonathan NiederNov 19, 2014
  16. Jeff KingNov 19, 2014
  17. Jonathan NiederNov 19, 2014
  18. Junio C HamanoNov 19, 2014
  19. Jeff KingNov 19, 2014
  20. Junio C HamanoNov 19, 2014
  21. Jeff KingNov 19, 2014
  22. Jonathan NiederNov 20, 2014
  23. 4/4 lock_ref_sha1_basic: do not die on locking errorsJeff King, Nov 19, 2014
  24. Jonathan NiederNov 19, 2014

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.