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.
