Re: [PATCH v7 04/33] files-backend: break out ref reading

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