Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@xxxxxxxxx> writes:
>> +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.
I could do that. It's less efficient but if the prevailing sentiment
is that it's worth it then I don't mind.
Would adding a comment to the implementation of strbuf_prefixf help?
> 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.
Yes, that's the idea.
I'll do the tc_err thing, since I'm not getting a clear feeling that
I've offered enough motivation for the prefixf approach.
Jonathan
--
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