Re: [PATCH 06/13] Dump the revprops at the start of every revision

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
Received on 2010-07-21 20:58:33 CEST