Re: [PATCH 2/3] strbuf: add strbuf_percentquote_buf

Jeff King [off-list ref] writes:

quoted

 (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]);
		}
        }

quoted

--- 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.