2018-01-27 0:05 GMT+03:00 Junio C Hamano <gitster@xxxxxxxxx>:
> 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.
I will add comments, agree, thank you.
>
>> -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.
I will change that piece of code by putting fwrite into if statement
also, thank you. We really do not put anything to buffer if retval is
not equal to zero (checked that).
Thank you for all your comments, waiting for further review.
Olga
