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