You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2014/01/02 15:01:43 UTC
svn commit: r1554807 - in /subversion/trunk/subversion: libsvn_fs/editor.c
libsvn_repos/delta.c libsvn_repos/rev_hunt.c mod_dav_svn/util.c
mod_dav_svn/version.c
Author: stefan2
Date: Thu Jan 2 14:01:43 2014
New Revision: 1554807
URL: http://svn.apache.org/r1554807
Log:
Replace the use of svn_fs_node_id + svn_fs_compare_ids with
simple calls to svn_fs_node_relation.
* subversion/libsvn_fs/editor.c
(can_modify): Simplify using the new API.
* subversion/libsvn_repos/delta.c
(svn_repos_dir_delta2): Ditto.
* subversion/mod_dav_svn/util.c
(dav_svn__get_safe_cr): Ditto.
* subversion/mod_dav_svn/version.c
(dav_svn__checkout): Ditto.
* subversion/libsvn_repos/rev_hunt.c
(svn_repos_trace_node_locations): Ditto.
(svn_repos_deleted_rev): Same, plus use svn_fs_check_path to test
for path validity instead of checking the
result of svn_fs_node_id.
Modified:
subversion/trunk/subversion/libsvn_fs/editor.c
subversion/trunk/subversion/libsvn_repos/delta.c
subversion/trunk/subversion/libsvn_repos/rev_hunt.c
subversion/trunk/subversion/mod_dav_svn/util.c
subversion/trunk/subversion/mod_dav_svn/version.c
Modified: subversion/trunk/subversion/libsvn_fs/editor.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs/editor.c?rev=1554807&r1=1554806&r2=1554807&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs/editor.c (original)
+++ subversion/trunk/subversion/libsvn_fs/editor.c Thu Jan 2 14:01:43 2014
@@ -240,23 +240,18 @@ can_modify(svn_fs_root_t *txn_root,
of those new revisions.
In either case, the node may not have changed in those new
revisions; use the node's ID to determine this case. */
- const svn_fs_id_t *txn_noderev_id;
svn_fs_root_t *rev_root;
- const svn_fs_id_t *new_noderev_id;
-
- /* The ID of the node that we would be modifying in the txn */
- SVN_ERR(svn_fs_node_id(&txn_noderev_id, txn_root, fspath,
- scratch_pool));
+ svn_fs_node_relation_t relation;
/* Get the ID from the future/new revision. */
SVN_ERR(svn_fs_revision_root(&rev_root, svn_fs_root_fs(txn_root),
revision, scratch_pool));
- SVN_ERR(svn_fs_node_id(&new_noderev_id, rev_root, fspath,
- scratch_pool));
+ SVN_ERR(svn_fs_node_relation(&relation, txn_root, fspath, rev_root,
+ fspath, scratch_pool));
svn_fs_close_root(rev_root);
/* Has the target node changed in the future? */
- if (svn_fs_compare_ids(txn_noderev_id, new_noderev_id) != 0)
+ if (relation != svn_fs_node_same)
{
/* Restarting the commit will base the txn on the future/new
revision, allowing the modification at REVISION. */
Modified: subversion/trunk/subversion/libsvn_repos/delta.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/delta.c?rev=1554807&r1=1554806&r2=1554807&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_repos/delta.c (original)
+++ subversion/trunk/subversion/libsvn_repos/delta.c Thu Jan 2 14:01:43 2014
@@ -216,10 +216,9 @@ svn_repos_dir_delta2(svn_fs_root_t *src_
void *root_baton = NULL;
struct context c;
const char *src_fullpath;
- const svn_fs_id_t *src_id, *tgt_id;
svn_node_kind_t src_kind, tgt_kind;
svn_revnum_t rootrev;
- int distance;
+ svn_fs_node_relation_t relation;
const char *authz_root_path;
/* SRC_PARENT_DIR must be valid. */
@@ -319,11 +318,10 @@ svn_repos_dir_delta2(svn_fs_root_t *src_
}
/* Get and compare the node IDs for the source and target. */
- SVN_ERR(svn_fs_node_id(&tgt_id, tgt_root, tgt_fullpath, pool));
- SVN_ERR(svn_fs_node_id(&src_id, src_root, src_fullpath, pool));
- distance = svn_fs_compare_ids(src_id, tgt_id);
+ SVN_ERR(svn_fs_node_relation(&relation, tgt_root, tgt_fullpath,
+ src_root, src_fullpath, pool));
- if (distance == 0)
+ if (relation == svn_fs_node_same)
{
/* They are the same node! No-op (you gotta love those). */
goto cleanup;
@@ -334,7 +332,7 @@ svn_repos_dir_delta2(svn_fs_root_t *src_
add the other. Also, if they are completely unrelated and
our caller is interested in relatedness, we do the same thing. */
if ((src_kind != tgt_kind)
- || ((distance == -1) && (! ignore_ancestry)))
+ || ((relation == svn_fs_node_unrelated) && (! ignore_ancestry)))
{
SVN_ERR(authz_root_check(tgt_root, authz_root_path,
authz_read_func, authz_read_baton, pool));
Modified: subversion/trunk/subversion/libsvn_repos/rev_hunt.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/rev_hunt.c?rev=1554807&r1=1554806&r2=1554807&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_repos/rev_hunt.c (original)
+++ subversion/trunk/subversion/libsvn_repos/rev_hunt.c Thu Jan 2 14:01:43 2014
@@ -309,11 +309,11 @@ svn_repos_deleted_rev(svn_fs_t *fs,
apr_pool_t *pool)
{
apr_pool_t *subpool;
- svn_fs_root_t *root, *copy_root;
+ svn_fs_root_t *start_root, *root, *copy_root;
const char *copy_path;
svn_revnum_t mid_rev;
- const svn_fs_id_t *start_node_id, *curr_node_id;
- svn_error_t *err;
+ svn_node_kind_t kind;
+ svn_fs_node_relation_t node_relation;
/* Validate the revision range. */
if (! SVN_IS_VALID_REVNUM(start))
@@ -334,32 +334,19 @@ svn_repos_deleted_rev(svn_fs_t *fs,
}
/* Ensure path exists in fs at start revision. */
- SVN_ERR(svn_fs_revision_root(&root, fs, start, pool));
- err = svn_fs_node_id(&start_node_id, root, path, pool);
- if (err)
+ SVN_ERR(svn_fs_revision_root(&start_root, fs, start, pool));
+ SVN_ERR(svn_fs_check_path(&kind, start_root, path, pool));
+ if (kind == svn_node_none)
{
- if (err->apr_err == SVN_ERR_FS_NOT_FOUND)
- {
- /* Path must exist in fs at start rev. */
- *deleted = SVN_INVALID_REVNUM;
- svn_error_clear(err);
- return SVN_NO_ERROR;
- }
- return svn_error_trace(err);
+ /* Path must exist in fs at start rev. */
+ *deleted = SVN_INVALID_REVNUM;
+ return SVN_NO_ERROR;
}
/* Ensure path was deleted at or before end revision. */
SVN_ERR(svn_fs_revision_root(&root, fs, end, pool));
- err = svn_fs_node_id(&curr_node_id, root, path, pool);
- if (err && err->apr_err == SVN_ERR_FS_NOT_FOUND)
- {
- svn_error_clear(err);
- }
- else if (err)
- {
- return svn_error_trace(err);
- }
- else
+ SVN_ERR(svn_fs_check_path(&kind, root, path, pool));
+ if (kind != svn_node_none)
{
/* path exists in the end node and the end node is equivalent
or otherwise equivalent to the start node. This can mean
@@ -386,8 +373,9 @@ svn_repos_deleted_rev(svn_fs_t *fs,
5) The start node was deleted and replaced by a node which
it does not share any history with.
*/
- SVN_ERR(svn_fs_node_id(&curr_node_id, root, path, pool));
- if (svn_fs_compare_ids(start_node_id, curr_node_id) != -1)
+ SVN_ERR(svn_fs_node_relation(&node_relation, start_root, path,
+ root, path, pool));
+ if (node_relation != svn_fs_node_unrelated)
{
SVN_ERR(svn_fs_closest_copy(©_root, ©_path, root,
path, pool));
@@ -450,28 +438,23 @@ svn_repos_deleted_rev(svn_fs_t *fs,
/* Get revision root and node id for mid_rev at that revision. */
SVN_ERR(svn_fs_revision_root(&root, fs, mid_rev, subpool));
- err = svn_fs_node_id(&curr_node_id, root, path, subpool);
-
- if (err)
+ SVN_ERR(svn_fs_check_path(&kind, root, path, pool));
+ if (kind == svn_node_none)
{
- if (err->apr_err == SVN_ERR_FS_NOT_FOUND)
- {
- /* Case D: Look lower in the range. */
- svn_error_clear(err);
- end = mid_rev;
- mid_rev = (start + mid_rev) / 2;
- }
- else
- return svn_error_trace(err);
+ /* Case D: Look lower in the range. */
+ end = mid_rev;
+ mid_rev = (start + mid_rev) / 2;
}
else
{
/* Determine the relationship between the start node
and the current node. */
- int cmp = svn_fs_compare_ids(start_node_id, curr_node_id);
+ SVN_ERR(svn_fs_node_relation(&node_relation, start_root, path,
+ root, path, pool));
+ if (node_relation != svn_fs_node_unrelated)
SVN_ERR(svn_fs_closest_copy(©_root, ©_path, root,
path, subpool));
- if (cmp == -1 ||
+ if (node_relation == svn_fs_node_unrelated ||
(copy_root &&
(svn_fs_revision_root_revision(copy_root) > start)))
{
@@ -654,7 +637,6 @@ svn_repos_trace_node_locations(svn_fs_t
svn_revnum_t revision;
svn_boolean_t is_ancestor;
apr_pool_t *lastpool, *currpool;
- const svn_fs_id_t *id;
SVN_ERR_ASSERT(location_revisions_orig->elt_size == sizeof(svn_revnum_t));
@@ -774,20 +756,22 @@ svn_repos_trace_node_locations(svn_fs_t
the node existing at the same path. We will look up path@lrev
for each remaining location-revision and make sure it is related
to path@revision. */
- SVN_ERR(svn_fs_revision_root(&root, fs, revision, currpool));
- SVN_ERR(svn_fs_node_id(&id, root, path, pool));
+ SVN_ERR(svn_fs_revision_root(&root, fs, revision, lastpool));
while (revision_ptr < revision_ptr_end)
{
svn_node_kind_t kind;
- const svn_fs_id_t *lrev_id;
+ svn_fs_node_relation_t node_relation;
+ svn_fs_root_t *cur_rev_root;
svn_pool_clear(currpool);
- SVN_ERR(svn_fs_revision_root(&root, fs, *revision_ptr, currpool));
- SVN_ERR(svn_fs_check_path(&kind, root, path, currpool));
+ SVN_ERR(svn_fs_revision_root(&cur_rev_root, fs, *revision_ptr,
+ currpool));
+ SVN_ERR(svn_fs_check_path(&kind, cur_rev_root, path, currpool));
if (kind == svn_node_none)
break;
- SVN_ERR(svn_fs_node_id(&lrev_id, root, path, currpool));
- if (! svn_fs_check_related(id, lrev_id))
+ SVN_ERR(svn_fs_node_relation(&node_relation, root, path,
+ cur_rev_root, path, currpool));
+ if (node_relation == svn_fs_node_unrelated)
break;
/* The node exists at the same path; record that and advance. */
Modified: subversion/trunk/subversion/mod_dav_svn/util.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/util.c?rev=1554807&r1=1554806&r2=1554807&view=diff
==============================================================================
--- subversion/trunk/subversion/mod_dav_svn/util.c (original)
+++ subversion/trunk/subversion/mod_dav_svn/util.c Thu Jan 2 14:01:43 2014
@@ -192,15 +192,9 @@ dav_svn__get_safe_cr(svn_fs_root_t *root
svn_revnum_t history_rev;
svn_fs_root_t *other_root;
svn_fs_t *fs = svn_fs_root_fs(root);
- const svn_fs_id_t *id, *other_id;
+ svn_fs_node_relation_t node_relation;
svn_error_t *err;
- if ((err = svn_fs_node_id(&id, root, path, pool)))
- {
- svn_error_clear(err);
- return revision; /* couldn't get id of root/path */
- }
-
if ((err = get_last_history_rev(&history_rev, root, path, pool)))
{
svn_error_clear(err);
@@ -213,13 +207,14 @@ dav_svn__get_safe_cr(svn_fs_root_t *root
return revision; /* couldn't open the history rev */
}
- if ((err = svn_fs_node_id(&other_id, other_root, path, pool)))
+ if ((err = svn_fs_node_relation(&node_relation, root, path,
+ other_root, path, pool)))
{
svn_error_clear(err);
- return revision; /* couldn't get id of other_root/path */
+ return revision;
}
- if (svn_fs_compare_ids(id, other_id) == 0)
+ if (node_relation == svn_fs_node_same)
return history_rev; /* the history rev is safe! the same node
exists at the same path in both revisions. */
Modified: subversion/trunk/subversion/mod_dav_svn/version.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/version.c?rev=1554807&r1=1554806&r2=1554807&view=diff
==============================================================================
--- subversion/trunk/subversion/mod_dav_svn/version.c (original)
+++ subversion/trunk/subversion/mod_dav_svn/version.c Thu Jan 2 14:01:43 2014
@@ -731,23 +731,12 @@ dav_svn__checkout(dav_resource *resource
revision than the one in the transaction. We'll
check to see if they are still the same node, and if
not, return an error. */
- const svn_fs_id_t *url_noderev_id, *txn_noderev_id;
-
- if ((serr = svn_fs_node_id(&txn_noderev_id, txn_root,
- resource->info->repos_path,
- resource->pool)))
- {
- err = dav_svn__new_error_svn
- (resource->pool, HTTP_CONFLICT, serr->apr_err,
- "Unable to fetch the node revision id of the version "
- "resource within the transaction");
- svn_error_clear(serr);
- return err;
- }
- if ((serr = svn_fs_node_id(&url_noderev_id,
- resource->info->root.root,
- resource->info->repos_path,
- resource->pool)))
+ svn_fs_node_relation_t node_relation;
+ if ((serr = svn_fs_node_relation(&node_relation, txn_root,
+ resource->info->repos_path,
+ resource->info->root.root,
+ resource->info->repos_path,
+ resource->pool)))
{
err = dav_svn__new_error_svn
(resource->pool, HTTP_CONFLICT, serr->apr_err,
@@ -756,7 +745,7 @@ dav_svn__checkout(dav_resource *resource
svn_error_clear(serr);
return err;
}
- if (svn_fs_compare_ids(url_noderev_id, txn_noderev_id) != 0)
+ if (node_relation != svn_fs_node_same)
{
return dav_svn__new_error_svn
(resource->pool, HTTP_CONFLICT, SVN_ERR_FS_CONFLICT,
Re: svn commit: r1554807 - in /subversion/trunk/subversion:
libsvn_fs/editor.c libsvn_repos/delta.c libsvn_repos/rev_hunt.c
mod_dav_svn/util.c mod_dav_svn/version.c
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Wed, Mar 4, 2015 at 7:06 PM, Ivan Zhakov <iv...@visualsvn.com> wrote:
> On 2 January 2014 at 17:01, <st...@apache.org> wrote:
> > Author: stefan2
> > Date: Thu Jan 2 14:01:43 2014
> > New Revision: 1554807
> >
> > URL: http://svn.apache.org/r1554807
> > Log:
> > Replace the use of svn_fs_node_id + svn_fs_compare_ids with
> > simple calls to svn_fs_node_relation.
> >
> Stefan,
>
> I was looking to one of compiler warnings and noticed unbounded memory
> usage problem in this commit. See my review inline.
>
> [...]
>
> > Modified: subversion/trunk/subversion/libsvn_repos/rev_hunt.c
> > URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/rev_hunt.c?rev=1554807&r1=1554806&r2=1554807&view=diff
> >
> ==============================================================================
> > --- subversion/trunk/subversion/libsvn_repos/rev_hunt.c (original)
> > +++ subversion/trunk/subversion/libsvn_repos/rev_hunt.c Thu Jan 2
> 14:01:43 2014
> > @@ -309,11 +309,11 @@ svn_repos_deleted_rev(svn_fs_t *fs,
> > apr_pool_t *pool)
> > {
> > apr_pool_t *subpool;
> > - svn_fs_root_t *root, *copy_root;
> > + svn_fs_root_t *start_root, *root, *copy_root;
> > const char *copy_path;
> > svn_revnum_t mid_rev;
> > - const svn_fs_id_t *start_node_id, *curr_node_id;
> > - svn_error_t *err;
> > + svn_node_kind_t kind;
> > + svn_fs_node_relation_t node_relation;
> >
> > /* Validate the revision range. */
> > if (! SVN_IS_VALID_REVNUM(start))
> > @@ -334,32 +334,19 @@ svn_repos_deleted_rev(svn_fs_t *fs,
> > }
> >
> > /* Ensure path exists in fs at start revision. */
> > - SVN_ERR(svn_fs_revision_root(&root, fs, start, pool));
> > - err = svn_fs_node_id(&start_node_id, root, path, pool);
> > - if (err)
> > + SVN_ERR(svn_fs_revision_root(&start_root, fs, start, pool));
> > + SVN_ERR(svn_fs_check_path(&kind, start_root, path, pool));
> > + if (kind == svn_node_none)
> > {
> > - if (err->apr_err == SVN_ERR_FS_NOT_FOUND)
> > - {
> > - /* Path must exist in fs at start rev. */
> > - *deleted = SVN_INVALID_REVNUM;
> > - svn_error_clear(err);
> > - return SVN_NO_ERROR;
> > - }
> > - return svn_error_trace(err);
> > + /* Path must exist in fs at start rev. */
> > + *deleted = SVN_INVALID_REVNUM;
> > + return SVN_NO_ERROR;
> > }
> >
> > /* Ensure path was deleted at or before end revision. */
> > SVN_ERR(svn_fs_revision_root(&root, fs, end, pool));
> > - err = svn_fs_node_id(&curr_node_id, root, path, pool);
> > - if (err && err->apr_err == SVN_ERR_FS_NOT_FOUND)
> > - {
> > - svn_error_clear(err);
> > - }
> > - else if (err)
> > - {
> > - return svn_error_trace(err);
> > - }
> > - else
> > + SVN_ERR(svn_fs_check_path(&kind, root, path, pool));
> > + if (kind != svn_node_none)
> > {
> > /* path exists in the end node and the end node is equivalent
> > or otherwise equivalent to the start node. This can mean
> > @@ -386,8 +373,9 @@ svn_repos_deleted_rev(svn_fs_t *fs,
> > 5) The start node was deleted and replaced by a node which
> > it does not share any history with.
> > */
> > - SVN_ERR(svn_fs_node_id(&curr_node_id, root, path, pool));
> > - if (svn_fs_compare_ids(start_node_id, curr_node_id) != -1)
> > + SVN_ERR(svn_fs_node_relation(&node_relation, start_root, path,
> > + root, path, pool));
> > + if (node_relation != svn_fs_node_unrelated)
> > {
> > SVN_ERR(svn_fs_closest_copy(©_root, ©_path, root,
> > path, pool));
> > @@ -450,28 +438,23 @@ svn_repos_deleted_rev(svn_fs_t *fs,
> >
> > /* Get revision root and node id for mid_rev at that revision. */
> > SVN_ERR(svn_fs_revision_root(&root, fs, mid_rev, subpool));
> > - err = svn_fs_node_id(&curr_node_id, root, path, subpool);
> > -
> > - if (err)
> > + SVN_ERR(svn_fs_check_path(&kind, root, path, pool));
> You're using wrong pool here: POOL instead of SUBPOOL (which is
> actually iterpool). As result this function consume unbounded amount
> of memory.
>
Apart from using the wrong pool and being a bug in that sense,
it should only be run O(log rev) times. Did you actually observe
unbound memory usage?
> > + if (kind == svn_node_none)
> > {
> > - if (err->apr_err == SVN_ERR_FS_NOT_FOUND)
> > - {
> > - /* Case D: Look lower in the range. */
> > - svn_error_clear(err);
> > - end = mid_rev;
> > - mid_rev = (start + mid_rev) / 2;
> > - }
> > - else
> > - return svn_error_trace(err);
> > + /* Case D: Look lower in the range. */
> > + end = mid_rev;
> > + mid_rev = (start + mid_rev) / 2;
> > }
> > else
> > {
> > /* Determine the relationship between the start node
> > and the current node. */
> > - int cmp = svn_fs_compare_ids(start_node_id, curr_node_id);
> > + SVN_ERR(svn_fs_node_relation(&node_relation, start_root, path,
> > + root, path, pool));
> Same here.
>
> > + if (node_relation != svn_fs_node_unrelated)
> > SVN_ERR(svn_fs_closest_copy(©_root, ©_path, root,
> > path, subpool));
> > - if (cmp == -1 ||
> > + if (node_relation == svn_fs_node_unrelated ||
> > (copy_root &&
> > (svn_fs_revision_root_revision(copy_root) > start)))
> > {
>
> I've fixed these problems in r1664084 and nominated to 1.9.x branch.
>
Yes, I've seen that commit. Thank you for fixing that.
-- Stefan^2.
Re: svn commit: r1554807 - in /subversion/trunk/subversion:
libsvn_fs/editor.c libsvn_repos/delta.c libsvn_repos/rev_hunt.c
mod_dav_svn/util.c mod_dav_svn/version.c
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 2 January 2014 at 17:01, <st...@apache.org> wrote:
> Author: stefan2
> Date: Thu Jan 2 14:01:43 2014
> New Revision: 1554807
>
> URL: http://svn.apache.org/r1554807
> Log:
> Replace the use of svn_fs_node_id + svn_fs_compare_ids with
> simple calls to svn_fs_node_relation.
>
Stefan,
I was looking to one of compiler warnings and noticed unbounded memory
usage problem in this commit. See my review inline.
[...]
> Modified: subversion/trunk/subversion/libsvn_repos/rev_hunt.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/rev_hunt.c?rev=1554807&r1=1554806&r2=1554807&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_repos/rev_hunt.c (original)
> +++ subversion/trunk/subversion/libsvn_repos/rev_hunt.c Thu Jan 2 14:01:43 2014
> @@ -309,11 +309,11 @@ svn_repos_deleted_rev(svn_fs_t *fs,
> apr_pool_t *pool)
> {
> apr_pool_t *subpool;
> - svn_fs_root_t *root, *copy_root;
> + svn_fs_root_t *start_root, *root, *copy_root;
> const char *copy_path;
> svn_revnum_t mid_rev;
> - const svn_fs_id_t *start_node_id, *curr_node_id;
> - svn_error_t *err;
> + svn_node_kind_t kind;
> + svn_fs_node_relation_t node_relation;
>
> /* Validate the revision range. */
> if (! SVN_IS_VALID_REVNUM(start))
> @@ -334,32 +334,19 @@ svn_repos_deleted_rev(svn_fs_t *fs,
> }
>
> /* Ensure path exists in fs at start revision. */
> - SVN_ERR(svn_fs_revision_root(&root, fs, start, pool));
> - err = svn_fs_node_id(&start_node_id, root, path, pool);
> - if (err)
> + SVN_ERR(svn_fs_revision_root(&start_root, fs, start, pool));
> + SVN_ERR(svn_fs_check_path(&kind, start_root, path, pool));
> + if (kind == svn_node_none)
> {
> - if (err->apr_err == SVN_ERR_FS_NOT_FOUND)
> - {
> - /* Path must exist in fs at start rev. */
> - *deleted = SVN_INVALID_REVNUM;
> - svn_error_clear(err);
> - return SVN_NO_ERROR;
> - }
> - return svn_error_trace(err);
> + /* Path must exist in fs at start rev. */
> + *deleted = SVN_INVALID_REVNUM;
> + return SVN_NO_ERROR;
> }
>
> /* Ensure path was deleted at or before end revision. */
> SVN_ERR(svn_fs_revision_root(&root, fs, end, pool));
> - err = svn_fs_node_id(&curr_node_id, root, path, pool);
> - if (err && err->apr_err == SVN_ERR_FS_NOT_FOUND)
> - {
> - svn_error_clear(err);
> - }
> - else if (err)
> - {
> - return svn_error_trace(err);
> - }
> - else
> + SVN_ERR(svn_fs_check_path(&kind, root, path, pool));
> + if (kind != svn_node_none)
> {
> /* path exists in the end node and the end node is equivalent
> or otherwise equivalent to the start node. This can mean
> @@ -386,8 +373,9 @@ svn_repos_deleted_rev(svn_fs_t *fs,
> 5) The start node was deleted and replaced by a node which
> it does not share any history with.
> */
> - SVN_ERR(svn_fs_node_id(&curr_node_id, root, path, pool));
> - if (svn_fs_compare_ids(start_node_id, curr_node_id) != -1)
> + SVN_ERR(svn_fs_node_relation(&node_relation, start_root, path,
> + root, path, pool));
> + if (node_relation != svn_fs_node_unrelated)
> {
> SVN_ERR(svn_fs_closest_copy(©_root, ©_path, root,
> path, pool));
> @@ -450,28 +438,23 @@ svn_repos_deleted_rev(svn_fs_t *fs,
>
> /* Get revision root and node id for mid_rev at that revision. */
> SVN_ERR(svn_fs_revision_root(&root, fs, mid_rev, subpool));
> - err = svn_fs_node_id(&curr_node_id, root, path, subpool);
> -
> - if (err)
> + SVN_ERR(svn_fs_check_path(&kind, root, path, pool));
You're using wrong pool here: POOL instead of SUBPOOL (which is
actually iterpool). As result this function consume unbounded amount
of memory.
> + if (kind == svn_node_none)
> {
> - if (err->apr_err == SVN_ERR_FS_NOT_FOUND)
> - {
> - /* Case D: Look lower in the range. */
> - svn_error_clear(err);
> - end = mid_rev;
> - mid_rev = (start + mid_rev) / 2;
> - }
> - else
> - return svn_error_trace(err);
> + /* Case D: Look lower in the range. */
> + end = mid_rev;
> + mid_rev = (start + mid_rev) / 2;
> }
> else
> {
> /* Determine the relationship between the start node
> and the current node. */
> - int cmp = svn_fs_compare_ids(start_node_id, curr_node_id);
> + SVN_ERR(svn_fs_node_relation(&node_relation, start_root, path,
> + root, path, pool));
Same here.
> + if (node_relation != svn_fs_node_unrelated)
> SVN_ERR(svn_fs_closest_copy(©_root, ©_path, root,
> path, subpool));
> - if (cmp == -1 ||
> + if (node_relation == svn_fs_node_unrelated ||
> (copy_root &&
> (svn_fs_revision_root_revision(copy_root) > start)))
> {
I've fixed these problems in r1664084 and nominated to 1.9.x branch.
---
Ivan Zhakov