Re: [PATCH RFC 02/24] ref-filter: add return value to some functions

Olga Telezhnaya [off-list ref] writes:

Add return flag to format_ref_array_item, show_ref_array_item,
get_ref_array_info and populate_value for further using.
Need it to handle situations when item is broken but we can not invoke
die() because we are in batch mode and all items need to be processed.

Signed-off-by: Olga Telezhnaia <redacted>
Mentored-by: Christian Couder [off-list ref]
Mentored by: Jeff King [off-list ref]
---
 ref-filter.c | 21 +++++++++++++--------
 ref-filter.h |  4 ++--
 2 files changed, 15 insertions(+), 10 deletions(-)

Makes sense as a preparatory step to pass the return status
throughout the call chain.  The functions that actually will detect
issues should probably gain

	/*
	 * a comment before the function
	 * that documents what it does and what values it returns
	 * to signal the failure.
	 */

before them; those that only pass the errorcode through to the
caller do not necessarily have to, though.

-void show_ref_array_item(struct ref_array_item *info,
+int show_ref_array_item(struct ref_array_item *info,
 			 const struct ref_format *format)
 {
 	struct strbuf final_buf = STRBUF_INIT;
+	int retval = format_ref_array_item(info, format, &final_buf);
 
-	format_ref_array_item(info, format, &final_buf);
 	fwrite(final_buf.buf, 1, final_buf.len, stdout);
 	strbuf_release(&final_buf);
-	putchar('\n');
+	if (!retval)
+		putchar('\n');
+	return retval;

This is questionable.  Why does it write final_buf out regardless of
the return value, even though it refrains from writing terminating LF
upon non-zero return from the same function that prepared final_buf?

"Because final_buf is left unmodified when formatter returns an
error, and calling fwrite on an empty buffer ends up being a no-op"
is a wrong answer---it relies on having too intimate knowledge on
how the callee happens to work currently.