On 09/16/2014 10:48 PM, Jonathan Nieder wrote:
Michael Haggerty wrote:
--- a/lockfile.c
+++ b/lockfile.c
@@ -219,13 +219,13 @@ int hold_lock_file_for_append(struct lock_file *lk,
const char *path, int flags)
if (errno != ENOENT) {
if (flags LOCK_DIE_ON_ERROR)
die(cannot open '%s' for copying, path);
-close(fd);
+rollback_lock_file(lk);
return error(cannot open '%s' for copying, path);
Makes sense.
Now that I'm here, I wonder a little at the error convention. If the
caller doesn't pass LOCK_DIE_ON_ERROR, are they supposed to be able to
use unable_to_lock_message? What errno would they pass in the err
parameter? Would callers want handle failure to acquire a lock
differently from other errors (e.g., by sleeping and trying again),
and if not, what is the optionally-die behavior in hold_lock_file
about?
The same applies to hold_lock_file_for_update(), so I'll discuss both at
once:
Most callers do pass LOCK_DIE_ON_ERROR. Of the ones that don't, a couple
appear to want to emit more meaningful error messages. A couple don't
die at all but return an error code to their caller. At least one
(add_to_alternates_file()) calls die_errno().
As it happens, hold_lock_file_for_append() sometimes overwrites errno
before it returns. I will add a patch on top of this series that fixes that.
I don't see any callers that retry, though I've thought about
implementing that in some places. But it's outside of the scope of this
patch series.
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
