2018-03-21 23:20 GMT+03:00 Junio C Hamano <gitster@xxxxxxxxx>:
> Olga Telezhnaya <olyatelezhnaya@xxxxxxxxx> writes:
>
>> Add function strbuf_error() that helps to save few lines of code.
>> Function expands fmt with placeholders, append resulting error message
>> to strbuf *err, and return error code ret.
>>
>> Signed-off-by: Olga Telezhnaia <olyatelezhnaya@xxxxxxxxx>
>> ---
>> strbuf.h | 13 +++++++++++++
>> 1 file changed, 13 insertions(+)
>>
>> diff --git a/strbuf.h b/strbuf.h
>> index e6cae5f4398c8..fa66d4835f1a7 100644
>> --- a/strbuf.h
>> +++ b/strbuf.h
>> @@ -620,4 +620,17 @@ char *xstrvfmt(const char *fmt, va_list ap);
>> __attribute__((format (printf, 1, 2)))
>> char *xstrfmt(const char *fmt, ...);
>>
>> +/*
>> + * Expand error message, append it to strbuf *err, then return error code ret.
>> + * Allow to save few lines of code.
>> + */
>> +static inline int strbuf_error(struct strbuf *err, int ret, const char *fmt, ...)
>> +{
>
> With this function, err does not have to be an error message, and
> ret does not have to be negative. Hence strbuf_error() is a wrong
> name for the wrapper.
>
> It somewhat is bothersome to see that this is inlined; if it is
> meant for error codepath, it probably shouldn't have to be.
>
>> + va_list ap;
>> + va_start(ap, fmt);
>> + strbuf_vaddf(err, fmt, ap);
>> + va_end(ap);
>> + return ret;
>> +}
>> +
>> #endif /* STRBUF_H */
>
> Quite honestly, I am not sure if it is worth to be in strbuf.h; it
> feels a bit too specific to the immediate need for these five
> patches and nowhere else. Are there many existing calls to
> strbuf_addf() immediately followed by an "return" of an integer,
> that can be simplified by using this helper? I see quite a many
> instances of addf() soon followed by "return -1" in
> refs/files-backend.c, but they are not immediately adjacent to each
> other, and won't be helped.
Summarizing all that we discussed: I have 2 options how to continue
this patch. I can revert to v4, or I can replace new function in
strbuf.h with similar macro in ref-filter.c (I mean, there would be no
changes in strbuf.h and one new macro in ref-filter.c). What do you
like more?
Thanks!
Olga
