Jonathan Nieder <jrnieder@xxxxxxxxx> writes:
> When preparing an error message in a strbuf, it can be convenient
> to add a formatted string to the beginning:
>
> if (transaction_commit(&t, err)) {
> strbuf_prefixf(err, "cannot fetch '%s': ", remotename);
> return -1;
> }
I am somewhat unhappy with this justification, as it is not very
clear why "cannot fetch '%s': " must come at the beginning without
knowing what kind of string transaction_commit() is expected to add
to err when it is called. It could be a sign that the convention
for transaction_commit() to report its errors is screwed up, and
not a sign that prefixf is necessary (not that I think that is the
case---there is not enough explanation here to decide).
> +void strbuf_prefixf(struct strbuf *sb, const char *fmt, ...)
> +{
> + va_list ap;
> + size_t pos, len;
> +
> + pos = sb->len;
> +
> + va_start(ap, fmt);
> + strbuf_vaddf(sb, fmt, ap);
> + va_end(ap);
> +
> + len = sb->len - pos;
> + strbuf_insert(sb, 0, sb->buf + pos, len);
> + strbuf_remove(sb, pos + len, len);
> +}
This indeed is strange to read; it would be more straightforward to
use a second strbuf for temporary storage you need to do this,
instead of using the tail-end of the original strbuf and shuffling
bytes around.
In any case, instead of this:
struct strbuf tc_err = STRBUF_INIT;
if (transaction_commit(&t, &tc_err)) {
strbuf_addf(err, "cannot fetch '%s': %s", remotename,
tc_err.buf);
strbuf_release(&tc_err);
return -1;
}
you can use the four-line version you cited above, which might be an
improvement. I dunno if it is such a big deal, though.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html