undef/git-commit-vandalism
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

lock_ref_sha1_basic: do not die on locking errors

Browse Source 
lock_ref_sha1_basic is inconsistent about when it calls
die() and when it returns NULL to signal an error. This is
annoying to any callers that want to recover from a locking
error.

This seems to be mostly historical accident. It was added in
4bd18c4 (Improve abstraction of ref lock/write.,
2006-05-17), which returned an error in all cases except
calling safe_create_leading_directories, in which case it
died.  Later, 40aaae8 (Better error message when we are
unable to lock the index file, 2006-08-12) asked
hold_lock_file_for_update to die for us, leaving the
resolve_ref code-path the only one which returned NULL.

We tried to correct that in 5cc3cef (lock_ref_sha1(): do not
sometimes error() and sometimes die()., 2006-09-30),
by converting all of the die() calls into returns. But we
missed the "die" flag passed to the lock code, leaving us
inconsistent. This state persisted until e5c223e
(lock_ref_sha1_basic(): if locking fails with ENOENT, retry,
2014-01-18). Because of its retry scheme, it does not ask
the lock code to die, but instead manually dies with
unable_to_lock_die().

We can make this consistent with the other return paths by
converting this to use unable_to_lock_message(), and
returning NULL. This is safe to do because all callers
already needed to check the return value of the function,
since it could fail (and return NULL) for other reasons.

[jk: Added excessive history explanation]

Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Ronnie Sahlberg 2014-11-19 17:28:52 -05:00 committed by Junio C Hamano
parent e69b1ce000
commit 068395150b
1 changed files with 8 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
refs.c
View File

@ -2318,6 +2318,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
lock->lock_fd = hold_lock_file_for_update(lock->lk, ref_file, lflags); lock->lock_fd = hold_lock_file_for_update(lock->lk, ref_file, lflags);
if (lock->lock_fd < 0) { if (lock->lock_fd < 0) {
last_errno = errno;
if (errno == ENOENT && --attempts_remaining > 0) if (errno == ENOENT && --attempts_remaining > 0)
/* /*
* Maybe somebody just deleted one of the * Maybe somebody just deleted one of the
@ -2325,8 +2326,13 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
* again: * again:
*/ */
goto retry; goto retry;
else else {
unable_to_lock_die(ref_file, errno); struct strbuf err = STRBUF_INIT;
unable_to_lock_message(ref_file, errno, &err);
error("%s", err.buf);
strbuf_reset(&err);
goto error_return;
}
} }
return old_sha1 ? verify_lock(lock, old_sha1, mustexist) : lock; return old_sha1 ? verify_lock(lock, old_sha1, mustexist) : lock;