Currently the svnrdump command line tool does something quite different compared
to other programs like svn, svnadmin and svnauthz. By "quite different" I mean
that svnrdump does exactly the following:
- Inconsistently uses (svn_cmdline_handle_exit_error) and SVNRDUMP_ERR,
which actually is (svn_handle_error2 + svn_error_clear) to handle errors.
- Sometimes destroys the top-level pool and sometimes does not destroy it, e.g.:
* Running 'svnrdump --non-interactive --force-interactive' explicitly destroys
* Running 'svnrdump dump INVALID-URL' does not destroy the pool thus leaving
it to be destroyed during apr_terminate.
- Prefixes every possible exit path with the svn_pool_destroy call instead of
making that call in a single place.
- Inconsistently uses SVN_INT_ERR instead of SVNRDUMP_ERR in one place:
if (subcommand->name == '-')
SVN_INT_ERR(help_cmd(NULL, NULL, pool));
We can avoid all these inconsistencies by utilizing the (main / submain /
SVN_INT_ERR / EXIT_ERROR / single svn_pool_destroy call) pattern. That is the
way things are done in in svn / svnadmin and svnauthz.
See the attached patch.
Another thing is worth mentioning here. This patch prevents svnrdump from
crashing in the SSL + error-out case, which I recently reported to serf-dev .
That is *just a lucky coincidence*, because the problem is in serf, not in
svnrdump. So, this patch should in no way be considered a workaround for the
serf issue, it is simply about making things more consistent and better in
Thanks and regards,
Received on 2013-08-18 02:28:36 CEST