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

[Junio C Hamano <gitster@pobox.com>]

Olga Telezhnaya <olyatelezhnaya@gmail.com> 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 <olyatelezhnaya@gmail.com>
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored by: Jeff King <peff@peff.net>
> ---
>  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.