On Mon, Apr 23, 2001 at 02:43:32PM -0000, cmpilato@tigris.org wrote:
>...
> + The caller must call editor->close_edit on EDIT_BATON;
> + svn_fs_dir_delta does not close the edit itself.
> +
> + Do any allocation necessary for the delta computation in POOL.
> + This function's maximum memory consumption is at most roughly
> + proportional to the greatest depth of SOURCE_PATH under
> + TARGET_ROOT, not the total size of the delta. */
> +svn_error_t *
> +svn_repos_update (svn_fs_root_t *target_root,
> + svn_fs_root_t *source_root,
> + svn_string_t *parent_dir,
> + svn_string_t *entry,
> + apr_hash_t *source_rev_diffs,
> + const svn_delta_edit_fns_t *editor,
> + void *edit_baton,
> + apr_pool_t *pool);
I don't understand the rationale for this function. Why do we have it, and
what is the key difference from svn_repos_dir_delta? Can that be clarified
in the documentation? They both seem to do the same thing, but with slightly
different arguments.
And *WHAT* is the reason for neither of them calling close_edit? Both of
them call set_target_revision and replace_root, so why not close_edit? It is
a pain in the butt for users of these functions to never touch the editor
*except* to have to go and clean up after the function, using a close_edit.
Why?
>...
> --- merge.c 2001/04/18 14:03:45 1.6
> +++ merge.c 2001/04/23 14:43:31 1.7
> @@ -402,9 +402,12 @@
> mrc.root = committed_root;
> mrc.repos = repos;
>
> - serr = svn_repos_dir_delta(previous_root, "/", revs,
> - committed_root, "/",
> + serr = svn_repos_dir_delta(previous_root,
> + svn_string_create ("/", pool), revs,
> + committed_root,
> + svn_string_create ("/", pool),
> editor, &mrc, pool);
I created a utility variable to hold a single "/" string here, rather than
needing to create two. Also, careful on the style (spaces).
Further, this whole thing for creating an svn_string_t for a *CONSTANT* is
*really* started to peeve me. The darned function isn't going to do any
modification of the string, so why use an svn_string_t?
I really think we need to reconsider our use of svn_string_t everywhere. It
is kind of silly to use it for constants. In many cases, we're just wrapping
these darn constants in the svn_string_t header plus copying the value
again. That is a lot less efficient than the occasional strlen().
The number of places where I need to do svn_string_create because of some
dumb API, when I *know* it doesn't need the svn_string_t memory buffering
gunk is growing, and is becoming somewhat silly.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Received on Sat Oct 21 14:36:29 2006