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
mhagger@xxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html