Re: [PATCH 2/3] strbuf: add strbuf_percentquote_buf
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:48:00
Subsystem:
the rest · Maintainer:
Linus Torvalds
Jeff King [off-list ref] writes:
quoted hunk ↗ jump to hunk
+`strbuf_percentquote_buf`:: + + Append the contents of one strbuf to another, quoting any + percent signs ("%") into double-percents ("%%") in the + destination. This is useful for literal data to be fed to either + strbuf_expand or to the *printf family of functions. + `strbuf_addf`:: Add a formatted string to the buffer.diff --git a/strbuf.c b/strbuf.c index 6cbc1fc..b5183c6 100644 --- a/strbuf.c +++ b/strbuf.c@@ -257,6 +257,16 @@ size_t strbuf_expand_dict_cb(struct strbuf *sb, const char *placeholder, return 0; } +void strbuf_percentquote_buf(struct strbuf *dest, struct strbuf *src) +{
Just a style thing, but please call that "dst" to be consistent. You are
already dropping vowels from the other side to spell it "src".
I wondered if the function should be just 1-arg that always quotes
in-place instead, but your [PATCH 3/3] wants to have an appending
semantics from this function, so changing it to be a 1-arg "in-place
quoter" will force the caller to run strbuf_addbuf() on the result, which
is not nice.
Since tucking a p-quoted version of the same string to its original
doesn't make sense at all, perhaps this should:
(0) be renamed to have "append" somewhere in its name;
(1) mark the src side as const; and
(2) perhaps have assert(dst != src). The loop won't terminate when
called with src == dst, I think.
There seems to be only one other strbuf function that takes two strbufs in
the suite (strbuf_addbuf), and I think it is unsafe in a different way,
which is trivial to fix.
-- >8 --
Subject: [PATCH] strbuf_addbuf(): allow passing the same buf to dst and src
If sb and sb2 are the same (i.e. doubling the string), the underlying
strbuf_add() will make sb2->buf invalid by calling strbuf_grow(sb) at
the beginning and will read from the freed buffer.
Signed-off-by: Junio C Hamano <redacted>
---
strbuf.h | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/strbuf.h b/strbuf.h
index fa07ecf..e272359 100644
--- 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); } extern void strbuf_adddup(struct strbuf *sb, size_t pos, size_t len);
--
1.6.6.280.ge295b7.dirty