On Wed, Mar 27, 2002 at 01:13:53PM -0600, sussman@tigris.org wrote:
>...
> * libsvn_client/repos_diff.c (svn_client__get_diff_editor): return
> new-style editor.
>
> (struct file_baton): remove pointer to parent dir_baton. in the
> new system, we need to account for the fact that file batons might
> outlive their parent dir batons. use a (const char *) for internal
Strictly speaking, file batons could have always outlived their parents.
>...
> +++ trunk/subversion/libsvn_client/repos_diff.c Wed Mar 27 13:13:49 2002
>...
> @@ -113,12 +115,14 @@
> svn_txdelta_window_handler_t apply_handler;
> void *apply_baton;
>
> - /* The baton for the parent directory. */
> - struct dir_baton *dir_baton;
> + /* The baton for the parent directory.
> + struct dir_baton *dir_baton; */
It's hard to recognize that this field is actually commented out. It took a
double-take to see what you'd done. I'd recommend just nuking this
altogether.
>...
> +/* Create a new directory baton in POOL. NAME is the directory name sans
> * path. ADDED is set if this directory is being added rather than
> * replaced. PARENT_BATON is the baton of the parent directory, it will be
> * null if this is the root of the comparison hierarchy. The directory and
> @@ -138,55 +142,47 @@
> * overall crawler editor baton.
> */
> static struct dir_baton *
> -make_dir_baton (const svn_stringbuf_t *name,
> +make_dir_baton (const char *name,
> struct dir_baton *parent_baton,
> struct edit_baton *edit_baton,
> svn_boolean_t added,
> apr_pool_t *pool)
> {
> - apr_pool_t *subpool = svn_pool_create (pool);
> - struct dir_baton *dir_baton = apr_pcalloc (subpool, sizeof (*dir_baton));
> + struct dir_baton *dir_baton = apr_pcalloc (pool, sizeof (*dir_baton));
>
> dir_baton->dir_baton = parent_baton;
> dir_baton->edit_baton = edit_baton;
> dir_baton->added = added;
> - dir_baton->pool = subpool;
> + dir_baton->pool = pool;
>
> - if (parent_baton)
> - {
> - dir_baton->path = svn_stringbuf_dup (parent_baton->path, dir_baton->pool);
> - }
> - else
> - {
> - dir_baton->path = svn_stringbuf_create ("", dir_baton->pool);
> - }
> + dir_baton->path = apr_pstrdup (pool,
> + parent_baton ? parent_baton->path : "");
>
> if (name)
> - svn_path_add_component (dir_baton->path, name);
> + dir_baton->path = svn_path_join (dir_baton->path, name, pool);
You're joining paths here, yet the callers already have paths. The callers
are yanking out the base name, just to pass it here to be joined(!).
The whole ball o' wax could be simplified by simply passing the full path
from the callers, into this function. The open_root() can pass "".
[ and on a (soon-to-be-moot) note, you're dup'ing "" for storage into a
'const char *'. You could simply do something like:
parent_baton ? apr_pstrdup(parent_baton->path) : ""
]
>...
> +/* Create a new file baton in POOL. NAME is the directory name sans
> + * path, which is a child of directory PARENT_PATH. ADDED is set if
> + * this file is being added rather than replaced. EDIT_BATON is a
> + * pointer to the global edit baton.
> */
> static struct file_baton *
> -make_file_baton (const svn_stringbuf_t *name,
> +make_file_baton (const char *name,
> svn_boolean_t added,
> - struct dir_baton *parent_baton)
> + const char *parent_path,
> + void *edit_baton,
> + apr_pool_t *pool)
> {
> - apr_pool_t *subpool = svn_pool_create (parent_baton->pool);
> - struct file_baton *file_baton = apr_pcalloc (subpool, sizeof (*file_baton));
> + struct file_baton *file_baton = apr_pcalloc (pool, sizeof (*file_baton));
>
> - file_baton->dir_baton = parent_baton;
> - file_baton->edit_baton = parent_baton->edit_baton;
> + file_baton->edit_baton = edit_baton;
> file_baton->added = added;
> - file_baton->pool = subpool;
> + file_baton->pool = pool;
>
> - file_baton->path = svn_stringbuf_dup (parent_baton->path, file_baton->pool);
> - svn_path_add_component (file_baton->path, name);
> + file_baton->path = svn_path_join (parent_path, name, pool);
And again: the callers are yanking out the base name, and then rejoining it
in this function. Just pass the full path and store it(!)
>...
> @@ -391,11 +385,13 @@
> case svn_node_file:
> {
> /* Compare a file being deleted against an empty file */
> - struct file_baton *b = make_file_baton (name, FALSE, pb);
> + struct file_baton *b = make_file_baton (svn_path_basename (path, pool),
> + FALSE, pb->path,
> + pb->edit_baton,
> + pool);
For example, right here. You could just pass 'path' and skip passing
pb->path.
>...
> @@ -425,7 +420,8 @@
>
> /* ### TODO: support copyfrom? */
>
> - b = make_dir_baton (name, pb, pb->edit_baton, TRUE, pb->pool);
> + b = make_dir_baton (svn_path_basename (path, pool),
> + pb, pb->edit_baton, TRUE, pool);
Hmm. Just realized that you're passing the parent_baton and the edit_baton.
You could just pass the parent and extract the edit baton from that.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Mar 27 22:02:48 2002