David Barr wrote: > [Subject: vcs-svn: factor out usage of string_pool] This actually means something like: use strbufs and strings instead of interned strings for values of rev, dump, and node fields that happen to be strings. After this change, there are no more users of the string-pool library left. > @@ -71,14 +71,16 @@ static void reset_rev_ctx(uint32_t revision) > rev_ctx.revision = revision; > rev_ctx.timestamp = 0; > strbuf_reset(&rev_ctx.log); > - rev_ctx.author = ~0; > + strbuf_reset(&rev_ctx.author); Side note: should the default timestamp really be the epoch? I'd rather the default timestamp be the timestamp of the parent revision, to make out-of-order dates a little less likely. > } > > -static void reset_dump_ctx(uint32_t url) > +static void reset_dump_ctx(const char *url) > { > - dump_ctx.url = url; > + strbuf_reset(&dump_ctx.url); > + if (url) > + strbuf_addstr(&dump_ctx.url, url); Good, we keep our own copy of the url still. > @@ -91,13 +93,15 @@ static void handle_property(const char *key, const char *val, uint32_t len, > break; > if (!val) > die("invalid dump: unsets svn:log"); > - /* Value length excludes terminating nul. */ > - strbuf_add(&rev_ctx.log, val, len + 1); > + strbuf_reset(&rev_ctx.log); > + strbuf_add(&rev_ctx.log, val, len); What is this change about? > @@ -447,5 +456,4 @@ void svndump_reset(void) > { > fast_export_reset(); > buffer_reset(&input); > - pool_reset(); strbuf_release(&dump_ctx.url)? Likewise for dump_ctx.uuid and the other one. Thanks; except as noted above this looks good. -- 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