On 04/15/2015 12:25 AM, Stefan Beller wrote:
> The 'lock_fd' is the same as 'lk->fd'. No need to store it twice so remove
> it. You may argue this introduces more coupling as we need to know more
> about the internals of the lock file mechanism, but this will be solved in
> a later patch.
>
> No functional changes intended.
This whole series LGTM; however, I suggest that this patch be split up.
See below.
> Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
> Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
> ---
> refs.c | 16 ++++++----------
> 1 file changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 14e52ca..4066752 100644
> --- a/refs.c
> +++ b/refs.c
> [...]
> @@ -2335,8 +2333,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
> goto error_return;
> }
>
> - lock->lock_fd = hold_lock_file_for_update(lock->lk, ref_file, lflags);
> - if (lock->lock_fd < 0) {
> + if (hold_lock_file_for_update(lock->lk, ref_file, lflags) < 0) {
> + last_errno = errno;
> if (errno == ENOENT && --attempts_remaining > 0)
> /*
> * Maybe somebody just deleted one of the
> [...]
Here you add the line "last_errno = errno". It is a good change, but it
is not part of removing ref_lock::lock_fd.
I suggest that you move this change to a separate commit. You might also
consider moving the new line to the "else" clause, because it's really
about preserving errno around the call to error() and preparing for
"goto error_return".
With or without this split, this patch is
Reviewed-by: Michael Haggerty <mhagger@xxxxxxxxxxxx>
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