Re: [PATCH v5 4/6] ref-filter: change parsing function error handling

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

Olga Telezhnaya <olyatelezhnaya@gmail.com> writes:

> @@ -2144,13 +2151,15 @@ int format_ref_array_item(struct ref_array_item *info,
>  
>  	for (cp = format->format; *cp && (sp = find_next(cp)); cp = ep + 1) {
>  		struct atom_value *atomv;
> +		int pos;
>  
>  		ep = strchr(sp, ')');
>  		if (cp < sp)
>  			append_literal(cp, sp, &state);
> -		get_ref_atom_value(info,
> -				   parse_ref_filter_atom(format, sp + 2, ep),
> -				   &atomv);
> +		pos = parse_ref_filter_atom(format, sp + 2, ep, error_buf);
> +		if (pos < 0)
> +			return -1;
> +		get_ref_atom_value(info, pos, &atomv);
>  		if (atomv->handler(atomv, &state, error_buf))
>  			return -1;
>  	}

These error returns leave the formatting state "state" on the stack
holding onto its resources, no?

The only thing the caller of format_ref_array_item() that notices an
error return does is to die even after this series, so in that sense
it does not matter (yet), but it still feels somewhat wrong.