On 04/02/2014 08:47 AM, Torsten Bögershausen wrote:
> []
>
> diff --git a/lockfile.c b/lockfile.c
> index c1af65b..1928e5e 100644
> --- a/lockfile.c
> +++ b/lockfile.c
> @@ -148,9 +148,11 @@ static int lock_file(struct lock_file *lk, const
> char *path, int flags)
> lock_file_list = lk;
> lk->on_list = 1;
> }
> - if (adjust_shared_perm(lk->filename))
> - return error("cannot fix permission bits on %s",
> - lk->filename);
> + if (adjust_shared_perm(lk->filename)) {
> + error("cannot fix permission bits on %s", lk->filename);
> + rollback_lock_file(lk);
> + return -1;
>
> Would it make sense to change the order of rollback() and error()?
> Make the rollback first (and as early as possible) and whine then?
Thanks for the feedback. It is a nice idea. But given that
rollback_lock_file() erases the filename field, making the change you
suggest would require us to make a copy before calling
rollback_lock_file(). The only advantage would be that we hold the lock
file for a moment less, right? Since this code path is only reached
when the repository has screwy permissions anyway, the next caller will
probably fail too. So the extra code complication doesn't seem worth it
to me.
Michael
--
Michael Haggerty
mhagger@xxxxxxxxxxxx
http://softwareswirl.blogspot.com/
--
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