Olga Telezhnaya <olyatelezhnaya@xxxxxxxxx> writes:
> Continue removing any printing from ref-filter formatting logic,
> so that it could be more general.
Hmm.
> Change the signature of parse_ref_filter_atom() by changing return value,
> adding previous return value to function parameter and also adding
> strbuf parameter for error message.
This says what the patch changes, but it does not explain why it is
a good idea to return something with different meaning to the
callers (which of course forces us to update all callers so that
they pass &result pointer and check the value returned in the
variable), or more importantly what meaning the return value has and
how the callers are expected to use it. While at it, it probably is
a good idea to explain what the original return value means.
The return value from parse_ref_filter_atom() used to be the
position the atom is found in the used_atom[] array; that
information is now returned in an integer pointed at by the
*res parameter. The function now returns 0 for success and
-1 for failure.
or something like that.
Having said that, I wonder if a calling convention that does not
force callers to pass in a pointer-to-int may make more sense.
Because the original return value is an index into an array, we know
the normal return values are not negative. An updated caller could
become like this instead:
pos = parse_ref_filter_atom(format, atom, ep, &err);
if (pos < 0)
die("%s", err.buf);
... original code that used 'pos' can stay as before ...
