Re: [PATCH] Re: "svn revert" performance

Gareth McCaughan <gareth.mccaughan@pobox.com> writes:

> On Saturday 01 November 2003 1:42 am, Philip Martin wrote:
>
> Yes, that's dumb. Will fix. I was led astray by the fact that the
> original code had a similar "feature": there's a line
>
> SVN_ERR (svn_wc_adm_close (adm_access));
>
> immediately before the sleep.

Our current code has bugs :)

>> Clear pools at the start of the loop.
>
> I was just following what revert-cmd.c does. What's the benefit
> of clearing at the start of the loop?

See the HACKING file. The project's knowledge about the use of pools
has developed over time, not all of the existing code follows the best
practice.

>> You don't appear to have deleted the subpool variable.
>
> Are we reading the same code? :-) The subpool is destroyed
> immediately on exit from the loop.

You don't appear to have deleted the declaration.

> 4. The name "out" for a clean-everything-up-and-return label
> is taken from the original revert.c code, but I don't like it. I'd
> prefer "finish" or something. Oh, and I'd like it painted dark
> green.

I dislike the way svn_client_revert uses labels irrespective of the
name, it's too easy to introduce a line "SVN_ERR (some_func())" that
isn't "label aware", witness my first comment above. When there is
some essential post-processing that must always be done I prefer to
use a wrapper function, see svn_repos_finish_report in
libsvn_repos/reporter.c.

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org