Hi Jonathan, I stashed this review away while working on some other important changes. I finally got around to responding to this review- sorry that it took so long. Jonathan Nieder writes: > > Fill in replay_revstart to dump the revprops at the start of every > > revision. Add an additional write_hash_to_stringbuf helper function. > > A write_hash_to_stringbuf helper does the work of > converting the property hashtable to dumpfile format. > > > +++ b/dumpr_util.c > [...] > > +void write_hash_to_stringbuf(apr_hash_t *properties, > > + svn_boolean_t deleted, > > + svn_stringbuf_t **strbuf, > > + apr_pool_t *pool) [...] Fixed, but not exactly in the way you've suggested. > > + /* Output name length, then name. */ > > + svn_stringbuf_appendcstr(*strbuf, > > + apr_psprintf(pool, "K %" APR_SSIZE_T_FMT "\n", > > + keylen)); > > + > > + svn_stringbuf_appendbytes(*strbuf, (const char *) key, keylen); > > Is the cast needed? (The answer might be "yes" if this code is meant > to be usable with C++ compilers.) These casts are all over in the source tree, so I'm guessing the answer is "yes". > Style: better to say in comments what we are trying to do than what we > actually do. So: > > /* First, dump revision properties. */ I've fixed all the comments in the entire source tree. Thanks :) -- Ram -- 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