You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Eric Gillespie <ep...@pretzelnet.org> on 2008/04/02 17:53:41 UTC
[PATCH] Call abort_edit upon editor error in repos reporter
[[[
Call abort_edit upon editor failure in the repos reporter.
* subversion/libsvn_repos/reporter.c
(drive): Don't call close_edit here.
(finish_report): If drive returns an error, call abort_edit (discarding
any error) and return the error. Else, call close_edit.
]]]
Index: subversion/libsvn_repos/reporter.c
===================================================================
--- subversion/libsvn_repos/reporter.c (revision 30158)
+++ subversion/libsvn_repos/reporter.c (working copy)
@@ -1179,9 +1179,7 @@ drive(report_baton_t *b, svn_revnum_t s_rev, path_
t_entry, root_baton, b->s_operand, info,
info->depth, b->requested_depth, pool));
- SVN_ERR(b->editor->close_directory(root_baton, pool));
- SVN_ERR(b->editor->close_edit(b->edit_baton, pool));
- return SVN_NO_ERROR;
+ return b->editor->close_directory(root_baton, pool);
}
/* Initialize the baton fields for editor-driving, and drive the editor. */
@@ -1239,7 +1237,13 @@ finish_report(report_baton_t *b, apr_pool_t *pool)
for (i = 0; i < NUM_CACHED_SOURCE_ROOTS; i++)
b->s_roots[i] = NULL;
- return drive(b, s_rev, info, pool);
+ {
+ svn_error_t *err = drive(b, s_rev, info, pool);
+ if (err == SVN_NO_ERROR)
+ return b->editor->close_edit(b->edit_baton, pool);
+ svn_error_clear(b->editor->abort_edit(b->edit_baton, pool));
+ return err;
+ }
}
/* --- COLLECTING THE REPORT INFORMATION --- */
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Call abort_edit upon editor error in repos reporter
Posted by Karl Fogel <kf...@red-bean.com>.
Eric Gillespie <ep...@pretzelnet.org> writes:
> I thought an entire error chain is always about the same error,
> but each error in the chain is more specific. This is a
> completely unrelated error. It would be very odd to have this
> one reporter to the user as an outermost error.
>
> Indeed, discarding it is the general pattern:
>
> [...]
I surrender.
> Thanks for reviewing these two; I don't hold it against you if
> you skip reviewing the swig-py nastiness ;->. Thing about that
> one is that my local Python expert found a larger reference
> counting issue in that code (which dates back to gstein in 2002!)
> when he revieed my change. I'll bring that one up next.
I did skip the swig-py patch, yes :-). But I'm curious to see about
this ref-counting issue, I'll look for it.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Call abort_edit upon editor error in repos reporter
Posted by Eric Gillespie <ep...@pretzelnet.org>.
Karl Fogel <kf...@red-bean.com> writes:
> Looks good, but why not chain any error returned by abort_edit(),
> instead of discarding it?
I thought an entire error chain is always about the same error,
but each error in the chain is more specific. This is a
completely unrelated error. It would be very odd to have this
one reporter to the user as an outermost error.
Indeed, discarding it is the general pattern:
libsvn_client/prop_commands.c:296: svn_error_clear(editor->abort_edit(edit_baton, pool));
libsvn_client/delete.c:216: svn_error_clear(editor->abort_edit(edit_baton, pool));
libsvn_client/copy.c:1035: svn_error_clear(editor->abort_edit(edit_baton, pool));
libsvn_client/commit_util.c:1739: return (*eb->real_editor->abort_edit)(eb->real_eb, pool);
libsvn_client/commit_util.c:1772: ed->abort_edit = abort_edit;
libsvn_client/add.c:773: svn_error_clear(editor->abort_edit(edit_baton, pool));
libsvn_client/commit.c:591: SVN_ERR(editor->abort_edit(edit_baton, pool));
libsvn_client/commit.c:795: svn_error_clear(editor->abort_edit(edit_baton, subpool));
libsvn_client/commit.c:1747: svn_error_clear(editor->abort_edit(edit_baton, pool));
libsvn_ra_neon/commit.c:1504: commit_editor->abort_edit = commit_abort_edit;
libsvn_ra_serf/commit.c:2052: editor->abort_edit = abort_edit;
libsvn_ra_svn/editorp.c:423: ra_svn_editor->abort_edit = ra_svn_abort_edit;
libsvn_ra_svn/editorp.c:788: SVN_CMD_ERR(ds->editor->abort_edit(ds->edit_baton, pool));
libsvn_ra_svn/editorp.c:902: svn_error_clear(editor->abort_edit(edit_baton, subpool));
libsvn_repos/commit.c:828: e->abort_edit = abort_edit;
svnserve/serve.c:653: svn_error_clear(editor->abort_edit(edit_baton, pool));
svnserve/serve.c:2329: svn_error_clear(editor->abort_edit(edit_baton, pool));
tests/libsvn_repos/repos-test.c:1593: SVN_ERR(editor->abort_edit(edit_baton, subpool));
tests/libsvn_repos/repos-test.c:1663: SVN_ERR(editor->abort_edit(edit_baton, subpool));
tests/svn_test_editor.c:572: my_editor->abort_edit = test_abort_edit;
Thanks for reviewing these two; I don't hold it against you if
you skip reviewing the swig-py nastiness ;->. Thing about that
one is that my local Python expert found a larger reference
counting issue in that code (which dates back to gstein in 2002!)
when he revieed my change. I'll bring that one up next.
--
Eric Gillespie <*> epg@pretzelnet.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Call abort_edit upon editor error in repos reporter
Posted by Karl Fogel <kf...@red-bean.com>.
Eric Gillespie <ep...@pretzelnet.org> writes:
> [[[
> Call abort_edit upon editor failure in the repos reporter.
>
> * subversion/libsvn_repos/reporter.c
> (drive): Don't call close_edit here.
> (finish_report): If drive returns an error, call abort_edit (discarding
> any error) and return the error. Else, call close_edit.
> ]]]
>
> Index: subversion/libsvn_repos/reporter.c
> ===================================================================
> --- subversion/libsvn_repos/reporter.c (revision 30158)
> +++ subversion/libsvn_repos/reporter.c (working copy)
> @@ -1179,9 +1179,7 @@ drive(report_baton_t *b, svn_revnum_t s_rev, path_
> t_entry, root_baton, b->s_operand, info,
> info->depth, b->requested_depth, pool));
>
> - SVN_ERR(b->editor->close_directory(root_baton, pool));
> - SVN_ERR(b->editor->close_edit(b->edit_baton, pool));
> - return SVN_NO_ERROR;
> + return b->editor->close_directory(root_baton, pool);
> }
>
> /* Initialize the baton fields for editor-driving, and drive the editor. */
> @@ -1239,7 +1237,13 @@ finish_report(report_baton_t *b, apr_pool_t *pool)
> for (i = 0; i < NUM_CACHED_SOURCE_ROOTS; i++)
> b->s_roots[i] = NULL;
>
> - return drive(b, s_rev, info, pool);
> + {
> + svn_error_t *err = drive(b, s_rev, info, pool);
> + if (err == SVN_NO_ERROR)
> + return b->editor->close_edit(b->edit_baton, pool);
> + svn_error_clear(b->editor->abort_edit(b->edit_baton, pool));
> + return err;
> + }
> }
>
> /* --- COLLECTING THE REPORT INFORMATION --- */
Looks good, but why not chain any error returned by abort_edit(),
instead of discarding it?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org