Subject: Re: [PATCH v5 07/35] hold_lock_file_for_append(): release lock on errors

From: Jonathan Nieder <[email-masked]>
Date: Tue, 16 Sep 2014 13:48:53 -0700

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?

In any case,
Reviewed-by: Jonathan Nieder <[email-masked]>