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

[PATCH v2 2/5] refs.c: do not die if locking fails in `write_pseudoref()`

From
MÅMartin Ågren <martin.agren@gmail.com>
Date
May 9, 2018, 20:55 UTC
Message-ID
<c1240491bc9029e2a72b0330a85820c1c272f537.1525898125.git.martin.agren@gmail.com>
In-Reply-To
<cover.1525898125.git.martin.agren@gmail.com>

If we could not take the lock, we add an error to the `strbuf err` and return. However, this code is dead. The reason is that we take the lock using `LOCK_DIE_ON_ERROR`. Drop the flag to allow our more gentle error-handling to actually kick in.

We could instead just drop the dead code and die here. But everything is prepared for gently propagating the error, so let's do that instead.

There is similar dead code in `delete_pseudoref()`, but let's save that for the next patch.

While at it, make the lock non-static. (Placing `struct lock_file`s on the stack used to be a bad idea, because the temp- and lockfile-machinery would keep a pointer into the struct. But after 076aa2cbd (tempfile: auto-allocate tempfiles on heap, 2017-09-05), we can safely have lockfiles on the stack.)

Reviewed-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 refs.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/refs.c b/refs.c
index 8b7a77fe5e..8c50b8b139 100644
--- a/refs.c
+++ b/refs.c
@@ -644,7 +644,7 @@ static int write_pseudoref(const char *pseudoref, const struct object_id *oid,
 {
 	const char *filename;
 	int fd;
-	static struct lock_file lock;
+	struct lock_file lock = LOCK_INIT;
 	struct strbuf buf = STRBUF_INIT;
 	int ret = -1;
 
@@ -654,8 +654,7 @@ static int write_pseudoref(const char *pseudoref, const struct object_id *oid,
 	strbuf_addf(&buf, "%s\n", oid_to_hex(oid));
 
 	filename = git_path("%s", pseudoref);
-	fd = hold_lock_file_for_update_timeout(&lock, filename,
-					       LOCK_DIE_ON_ERROR,
+	fd = hold_lock_file_for_update_timeout(&lock, filename, 0,
 					       get_files_ref_lock_timeout_ms());
 	if (fd < 0) {
 		strbuf_addf(err, "could not open '%s' for writing: %s",
-- 
2.17.0.411.g9fd64c8e46
Previous: Martin ÅgrenNext: Martin Ågren
Message 5 of 11 in “getting rid of most "static struct lock_file"s”
  1. 0/5 getting rid of most "static struct lock_file"sMartin Ågren, May 6, 2018
  2. Jeff KingMay 8, 2018
  3. 0/5 getting rid of most "static struct lock_file"sMartin Ågren, May 9, 2018
  4. 1/5 t/helper/test-write-cache: clean up lock-handlingMartin Ågren, May 9, 2018
  5. 2/5 refs.c: do not die if locking fails in `write_pseudoref()`Martin Ågren, May 9, 2018
  6. 3/5 refs.c: do not die if locking fails in `delete_pseudoref()`Martin Ågren, May 9, 2018
  7. 4/5 lock_file: make function-local locks non-staticMartin Ågren, May 9, 2018
  8. 5/5 lock_file: move static locks into functionsMartin Ågren, May 9, 2018
  9. Jeff KingMay 10, 2018
  10. Junio C HamanoMay 10, 2018
  11. Martin ÅgrenMay 10, 2018

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.