Jeff King <peff@xxxxxxxx> writes:
>> (2) perhaps have assert(dst != src). The loop won't terminate when
>> called with src == dst, I think.
>
> Oops again. I think it is sensible to protect against this. I thought
> about trying to make it magically work in-place, but I don't think there
> is a simple way to do so.
As I said, I don't think appending p-quoted version of itself to a string
makes much sense, but I don't think in-place is too difficult.
strbuf_addbuf_pquote(*dst, *src)
{
int len = src->len, i;
for (i = 0; i < len; i++) {
if (src->buf[i] == '%')
strbuf_addch(dst, '%');
strbuf_addch(dst, src->buf[i]);
}
}
>> --- a/strbuf.h
>> +++ b/strbuf.h
>> @@ -105,7 +105,13 @@ static inline void strbuf_addstr(struct strbuf *sb, const char *s) {
>> strbuf_add(sb, s, strlen(s));
>> }
>> static inline void strbuf_addbuf(struct strbuf *sb, const struct strbuf *sb2) {
>> - strbuf_add(sb, sb2->buf, sb2->len);
>> + char *buf = sb2->buf;
>> + int len = sb2->len;
>> + if (sb->buf == sb2->buf) {
>> + strbuf_grow(sb, len);
>> + buf = sb->buf;
>> + }
>> + strbuf_add(sb, buf, len);
>> }
>
> Shouldn't this be "if (sb == sb2)"? Two strbufs in the initial state
> will point to the same strbuf_slopbuf, but obviously growing sb will not
> impact sb2. Though that would simply provoke a false positive, which I
> don't think has any negative consequences.
Ok, that is a good catch. And two strbufs that share the same allocated
string is a user error
> Also, since reallocating sb will reallocate sb2, can't you just write it
> safely like this:
>
> strbuf_grow(sb, sb2->len);
> strbuf_add(sb, sb2->buf, sb2->len);
I didn't want to worry about a semi-clever (read: broken) compilers doing
semi-clever things assuming sb and sb2 do not alias, but I agree that your
approach is much simpler.
Thanks.
--
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