Junio C Hamano wrote:
> Jonathan Nieder <jrnie...@gmail.com> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to