From: "Martin Ågren" <martin.agren@gmail.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, Duy Nguyen <pclouds@gmail.com>,
David Turner <novalis@novalis.org>,
Stefan Beller <sbeller@google.com>
Subject: [PATCH v2 2/5] refs.c: do not die if locking fails in `write_pseudoref()`
Date: Wed, 9 May 2018 22:55:36 +0200 [thread overview]
Message-ID: <c1240491bc9029e2a72b0330a85820c1c272f537.1525898125.git.martin.agren@gmail.com> (raw)
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
next prev parent reply other threads:[~2018-05-09 20:56 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-05-06 14:10 [PATCH 0/5] getting rid of most "static struct lock_file"s Martin Ågren
2018-05-08 18:25 ` Jeff King
2018-05-09 20:55 ` [PATCH v2 " Martin Ågren
2018-05-09 20:55 ` [PATCH v2 1/5] t/helper/test-write-cache: clean up lock-handling Martin Ågren
2018-05-09 20:55 ` Martin Ågren [this message]
2018-05-09 20:55 ` [PATCH v2 3/5] refs.c: do not die if locking fails in `delete_pseudoref()` Martin Ågren
2018-05-09 20:55 ` [PATCH v2 4/5] lock_file: make function-local locks non-static Martin Ågren
2018-05-09 20:55 ` [PATCH v2 5/5] lock_file: move static locks into functions Martin Ågren
2018-05-10 5:21 ` [PATCH v2 0/5] getting rid of most "static struct lock_file"s Jeff King
2018-05-10 6:01 ` Junio C Hamano
2018-05-10 7:47 ` Martin Ågren
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c1240491bc9029e2a72b0330a85820c1c272f537.1525898125.git.martin.agren@gmail.com \
--to=martin.agren@gmail.com \
--cc=git@vger.kernel.org \
--cc=novalis@novalis.org \
--cc=pclouds@gmail.com \
--cc=peff@peff.net \
--cc=sbeller@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).