Skip to content

Commit 3c6fad4

Browse files
Martin Ågrengitster
authored andcommitted
refs.c: do not die if locking fails in delete_pseudoref()
After taking the lock we check whether we got it and die otherwise. But since we take the lock using `LOCK_DIE_ON_ERROR`, we would already have died. Considering the choice between dropping the dead code and dropping the flag, let's go for option number three: Drop the flag, write an error instead of dying, then return -1. This function already returns -1 for another error, so the caller (or rather, its callers) should be able to handle this. There is some inconsistency around how we handle errors in this function and elsewhere in this file, but let's take this small step towards gentle error-reporting now and leave the rest for another time. While at it, make the lock non-static and reduce its scope. (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 076aa2c (tempfile: auto-allocate tempfiles on heap, 2017-09-05), we can safely have lockfiles on the stack.) Signed-off-by: Martin Ågren <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 0108451 commit 3c6fad4

File tree

1 file changed

+7
-4
lines changed

1 file changed

+7
-4
lines changed

refs.c

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -689,20 +689,23 @@ static int write_pseudoref(const char *pseudoref, const struct object_id *oid,
689689

690690
static int delete_pseudoref(const char *pseudoref, const struct object_id *old_oid)
691691
{
692-
static struct lock_file lock;
693692
const char *filename;
694693

695694
filename = git_path("%s", pseudoref);
696695

697696
if (old_oid && !is_null_oid(old_oid)) {
697+
struct lock_file lock = LOCK_INIT;
698698
int fd;
699699
struct object_id actual_old_oid;
700700

701701
fd = hold_lock_file_for_update_timeout(
702-
&lock, filename, LOCK_DIE_ON_ERROR,
702+
&lock, filename, 0,
703703
get_files_ref_lock_timeout_ms());
704-
if (fd < 0)
705-
die_errno(_("Could not open '%s' for writing"), filename);
704+
if (fd < 0) {
705+
error_errno(_("could not open '%s' for writing"),
706+
filename);
707+
return -1;
708+
}
706709
if (read_ref(pseudoref, &actual_old_oid))
707710
die("could not read ref '%s'", pseudoref);
708711
if (oidcmp(&actual_old_oid, old_oid)) {

0 commit comments

Comments
 (0)