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 076aa2cbd (tempfile: auto-allocate tempfiles on heap, 2017-09-05), we can safely have lockfiles on the stack.) Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Martin Ågren committed May 9, 2018 at 22:55 UTC 3c6fad4a3fcc9a01dd3d9678360907271ad85920
1 file changed +7 -4
refs.c
+7 -4
@@ -689,20 +689,23 @@ done:
689
690 static int delete_pseudoref(const char *pseudoref, const struct object_id *old_oid)
691 {
692 - static struct lock_file lock;
692 const char *filename;
693
694 filename = git_path("%s", pseudoref);
695
696 if (old_oid && !is_null_oid(old_oid)) {
697 + struct lock_file lock = LOCK_INIT;
698 int fd;
699 struct object_id actual_old_oid;
700
701 fd = hold_lock_file_for_update_timeout(
702 - &lock, filename, LOCK_DIE_ON_ERROR,
702 + &lock, filename, 0,
703 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 + }
709 if (read_ref(pseudoref, &actual_old_oid))
710 die("could not read ref '%s'", pseudoref);
711 if (oidcmp(&actual_old_oid, old_oid)) {