On 06/10/2014 12:19 AM, Jeremiah Mahler wrote: > Currently, the data in a strbuf is modified using add operations. To > set the buffer to some data a reset must be performed before an add. > > strbuf_reset(buf); > strbuf_add(buf, cb.buf.buf, cb.buf.len); > > And this is a common sequence of operations with 70 occurrences found in > the current source code. This includes all the different variations > (add, addf, addstr, addbuf, addch). > > FILES=`find ./ -name '*.c'` > CNT=$(pcregrep -M "strbuf_reset.*\n.*strbuf_add" $FILES | wc -l) > CNT=$(echo "$CNT / 2" | bc) > echo $CNT > 70 > > These patches add strbuf_set operations which allow this common sequence > to be performed in one line instead of two. > > strbuf_set(buf, cb.buf.buf, cb.buf.len); > > Signed-off-by: Jeremiah Mahler <jmmah...@gmail.com> > --- > Documentation/technical/api-strbuf.txt | 18 ++++++++++++++++++ > strbuf.c | 21 +++++++++++++++++++++ > strbuf.h | 14 ++++++++++++++ > 3 files changed, 53 insertions(+) > > diff --git a/Documentation/technical/api-strbuf.txt > b/Documentation/technical/api-strbuf.txt > index 077a709..b7e23da 100644 > --- a/Documentation/technical/api-strbuf.txt > +++ b/Documentation/technical/api-strbuf.txt > @@ -149,6 +149,24 @@ Functions > than zero if the first buffer is found, respectively, to be less than, > to match, or be greater than the second buffer. > > +* Setting the buffer > + > +`strbuf_set`:: > + > + Replace the buffer content with data of a given length. > + > +`strbuf_setstr`:: > + > + Replace the buffer content with data from a NUL-terminated string. > + > +`strbuf_setf`:: > + > + Replace the buffer content with a formatted string. > + > +`strbuf_setbuf`:: > + > + Replace the buffer content with data from another buffer. > + > * Adding data to the buffer > > NOTE: All of the functions in this section will grow the buffer as necessary. > diff --git a/strbuf.c b/strbuf.c > index ac62982..9d64b00 100644 > --- a/strbuf.c > +++ b/strbuf.c > @@ -189,6 +189,27 @@ void strbuf_splice(struct strbuf *sb, size_t pos, size_t > len, > strbuf_setlen(sb, sb->len + dlen - len); > } > > +void strbuf_set(struct strbuf *sb, const void *data, size_t len) > +{ > + strbuf_reset(sb); > + strbuf_add(sb, data, len); > +} > +
I never know how much intelligence to attribute to modern compilers, but it seems like even after optimization this function will be doing more work than necessary: strbuf_reset(sb) -> strbuf_setlen(sb, 0) -> sb->len = 0 -> sb->buf[len] = '\0' -> strbuf_add(sb, data, len) -> strbuf_grow(sb, len) -> ...lots of stuff... -> memcpy(sb->buf + sb->len, data, len) -> strbuf_setlen(sb, sb->len + len) -> sb->len = len -> sb->buf[len] = '\0' If there were a function like strbuf_grow_to(sb, len): void strbuf_grow_to(struct strbuf *sb, size_t len) { int new_buf = !sb->alloc; if (unsigned_add_overflows(len, 1)) die("you want to use way too much memory"); if (new_buf) sb->buf = NULL; ALLOC_GROW(sb->buf, len + 1, sb->alloc); if (new_buf) sb->buf[0] = '\0'; } (strbuf_grow() could call it: static inline void strbuf_grow(struct strbuf *sb, size_t extra) { if (unsigned_add_overflows(sb->len, extra)) die("you want to use way too much memory"); strbuf_grow_to(sb, sb->len + extra); } ), then your function could be minimized to void strbuf_set(struct strbuf *sb, const void *data, size_t len) { strbuf_grow_to(sb, len); memcpy(sb->buf, data, len); strbuf_setlen(sb, len); } I think strbuf_grow_to() would be useful in other situations too. This is just an idea; I'm not saying that you have to make this change. > [...] Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- 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