refs: make explicit that ref_iterator_peel returns boolean

hanwen · gitster · commit 617480d75bdc · 2021-05-20T07:54:12.000+09:00
Use -1 as error return value throughout.

This removes spurious differences in the GIT_TRACE_REFS output, depending on the
ref storage backend active.

Before, the cached ref_iterator (but only that iterator!) would return
peel_object() output directly. No callers relied on the peel_status values
beyond success/failure. All calls to these functions go through
peel_iterated_oid(), which returns peel_object() as a fallback, but also
squashing the error values.

The iteration interface already passes REF_ISSYMREF and REF_ISBROKEN through the
flags argument, so the additional error values in enum peel_status provide no
value.

The ref iteration interface provides a separate peel() function because certain
formats (eg. packed-refs and reftable) can store the peeled object next to the
tag SHA1. Passing the peeled SHA1 as an optional argument to each_ref_fn maps
more naturally to the implementation of ref databases. Changing the code in this
way is left for a future refactoring.

Signed-off-by: Han-Wen Nienhuys &lt;hanwen@google.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
diff --git a/refs.c b/refs.c
@@ -2010,7 +2010,7 @@ int peel_iterated_oid(const struct object_id *base, struct object_id *peeled)
 	     oideq(current_ref_iter->oid, base)))
 		return ref_iterator_peel(current_ref_iter, peeled);
 
-	return peel_object(base, peeled);
+	return peel_object(base, peeled) ? -1 : 0;
 }
 
 int refs_create_symref(struct ref_store *refs,
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
@@ -889,7 +889,7 @@ static int packed_ref_iterator_peel(struct ref_iterator *ref_iterator,
 	} else if ((iter->base.flags & (REF_ISBROKEN | REF_ISSYMREF))) {
 		return -1;
 	} else {
-		return !!peel_object(&iter->oid, peeled);
+		return peel_object(&iter->oid, peeled) ? -1 : 0;
 	}
 }
 
diff --git a/refs/ref-cache.c b/refs/ref-cache.c
@@ -491,7 +491,7 @@ static int cache_ref_iterator_advance(struct ref_iterator *ref_iterator)
 static int cache_ref_iterator_peel(struct ref_iterator *ref_iterator,
 				   struct object_id *peeled)
 {
-	return peel_object(ref_iterator->oid, peeled);
+	return peel_object(ref_iterator->oid, peeled) ? -1 : 0;
 }
 
 static int cache_ref_iterator_abort(struct ref_iterator *ref_iterator)
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
@@ -453,6 +453,9 @@ void base_ref_iterator_free(struct ref_iterator *iter);
  */
 typedef int ref_iterator_advance_fn(struct ref_iterator *ref_iterator);
 
+/*
+ * Peels the current ref, returning 0 for success or -1 for failure.
+ */
 typedef int ref_iterator_peel_fn(struct ref_iterator *ref_iterator,
 				 struct object_id *peeled);
 

Original file line number	Diff line number	Diff line change
`@@ -2010,7 +2010,7 @@ int peel_iterated_oid(const struct object_id base, struct object_id peeled)`
`2010`	`2010`	`oideq(current_ref_iter->oid, base)))`
`2011`	`2011`	`return ref_iterator_peel(current_ref_iter, peeled);`
`2012`	`2012`
`2013`		`- return peel_object(base, peeled);`
	`2013`	`+ return peel_object(base, peeled) ? -1 : 0;`
`2014`	`2014`	`}`
`2015`	`2015`
`2016`	`2016`	`int refs_create_symref(struct ref_store *refs,`
Original file line number	Diff line number	Diff line change
`@@ -889,7 +889,7 @@ static int packed_ref_iterator_peel(struct ref_iterator *ref_iterator,`
`889`	`889`	`} else if ((iter->base.flags & (REF_ISBROKEN \| REF_ISSYMREF))) {`
`890`	`890`	`return -1;`
`891`	`891`	`} else {`
`892`		`- return !!peel_object(&iter->oid, peeled);`
	`892`	`+ return peel_object(&iter->oid, peeled) ? -1 : 0;`
`893`	`893`	`}`
`894`	`894`	`}`
`895`	`895`
Original file line number	Diff line number	Diff line change
`@@ -491,7 +491,7 @@ static int cache_ref_iterator_advance(struct ref_iterator *ref_iterator)`
`491`	`491`	`static int cache_ref_iterator_peel(struct ref_iterator *ref_iterator,`
`492`	`492`	`struct object_id *peeled)`
`493`	`493`	`{`
`494`		`- return peel_object(ref_iterator->oid, peeled);`
	`494`	`+ return peel_object(ref_iterator->oid, peeled) ? -1 : 0;`
`495`	`495`	`}`
`496`	`496`
`497`	`497`	`static int cache_ref_iterator_abort(struct ref_iterator *ref_iterator)`