From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Sun, 20 Mar 2016 05:03:09 GMT
Subject: Re: [PATCH v7 04/33] files-backend: break out ref reading
Message-ID: <56EE2F0D.5030709@alum.mit.edu>
In-Reply-To: <1456793586-22082-5-git-send-email-dturner@twopensource.com>

On 03/01/2016 01:52 AM, David Turner wrote:
> Refactor resolve_ref_1 in terms of a new function read_raw_ref, which
> is responsible for reading ref data from the ref storage.
> 
> Later, we will make read_raw_ref a pluggable backend function, and make
> resolve_ref_unsafe common.
> 
> Testing done: Hacked in code to run both old and new version of
> resolve_ref_1 and compare all outputs, failing dramatically if outputs
> differed.  Ran test suite.

I like that you are splitting up resolve_ref_1(), which was too
complicated and convoluted before.

This is a textually large change and I'm still auditing it, but
meanwhile I have a question...

> [...]
> -		if (--depth < 0) {
> -			errno = ELOOP;
> -			return NULL;
> -		}

The old version set errno to ELOOP if there were too many layers of
symrefs. The new version doesn't seem to set errno at all in that case.
I think that is a regression, though I might be misunderstanding something.

Michael