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

From: Оля Тележная <olyatelezhnaya@gmail.com>
Date: Mar 23, 2018, 06:56 UTC
Message-ID: <CAL21BmnJ8ktMxt2-sLBE-US-j8p_MFGOYjdoCHPmdL0-NG=omg@mail.gmail.com>
In-Reply-To: <xmqqh8p9np2a.fsf@gitster-ct.c.googlers.com>

2018-03-21 23:36 GMT+03:00 Junio C Hamano <gitster@pobox.com>:

Show 24 quoted lines

> 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?

Yes, you are right, thanks a lot!

Show 5 quoted lines

>
> 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.
>

I am sure I need to fix it anyway, thanks.