You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Neels Janosch Hofmeyr <ne...@elego.de> on 2008/06/05 01:14:19 UTC
use NULL commit callback in svn_repos_get_commit_editor5?
Hi dev,
I see that it is mandatory to pass a non-NULL commit callback function
for svn_repos_get_commit_editor5, since otherwise a
editor->close_edit(..) aborts with a segmentation fault :(
Writing a C test, I just wanted nothing to happen when the commit is
done, but still had to write a dummy commit callback that does nothing
to avoid the segmentation fault.
Am I not getting something?
Thanks
--
Neels Hofmeyr -- elego Software Solutions GmbH
Gustav-Meyer-Allee 25 / Gebäude 12, 13355 Berlin, Germany
phone: +49 30 23458696 mobile: +49 177 2345869 fax: +49 30 23458695
http://www.elegosoft.com | Geschäftsführer: Olaf Wagner | Sitz: Berlin
Handelsreg: Amtsgericht Charlottenburg HRB 77719 | USt-IdNr: DE163214194
Re: use NULL commit callback in svn_repos_get_commit_editor5?
Posted by David Glasser <gl...@davidglasser.net>.
On Wed, Jun 4, 2008 at 11:45 PM, Karl Fogel <kf...@red-bean.com> wrote:
> "David Glasser" <gl...@davidglasser.net> writes:
>> I dunno, but looking at the original code it looks like there's an
>> error leak if either svn_fs_revision_prop call fails. And the call to
>> svn_error_clear(err), as well as the final "return err;", seem wrong,
>> since err should be guaranteed to be SVN_NO_ERROR by that point. Am I
>> missing something?
>
> You mean the existing code, before the patch? Yes, it looks wonky to me
> too. Fixing that up would be a good idea, but I'm not going to attempt
> it at 3am... :-)
Yeah, I mean the existing code: I was checking your patch for an error
leak and went "guh?".
--dave
--
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: use NULL commit callback in svn_repos_get_commit_editor5?
Posted by Karl Fogel <kf...@red-bean.com>.
"David Glasser" <gl...@davidglasser.net> writes:
> I dunno, but looking at the original code it looks like there's an
> error leak if either svn_fs_revision_prop call fails. And the call to
> svn_error_clear(err), as well as the final "return err;", seem wrong,
> since err should be guaranteed to be SVN_NO_ERROR by that point. Am I
> missing something?
You mean the existing code, before the patch? Yes, it looks wonky to me
too. Fixing that up would be a good idea, but I'm not going to attempt
it at 3am... :-)
-Karl
>> [[[
>> Improve an API by allowing a callback to be null.
>>
>> Suggested by: Neels Janosch Hofmeyr <ne...@elego.de>
>>
>> * subversion/include/svn_repos.h
>> (svn_repos_get_commit_editor5): Document that callback can be null.
>> Also, fix up some obsolete documentation: the modern callback type
>> takes a struct instead of separate parameters.
>>
>> * subversion/libsvn_repos/commit.c
>> (close_edit): Test callback before calling.
>> ]]]
>>
>> Index: subversion/include/svn_repos.h
>> ===================================================================
>> --- subversion/include/svn_repos.h (revision 31587)
>> +++ subversion/include/svn_repos.h (working copy)
>> @@ -938,15 +938,16 @@
>> * due to authz will return SVN_ERR_AUTHZ_UNREADABLE or
>> * SVN_ERR_AUTHZ_UNWRITABLE.
>> *
>> - * Calling @a (*editor)->close_edit completes the commit. Before
>> - * @c close_edit returns, but after the commit has succeeded, it will
>> - * invoke @a callback with the new revision number, the commit date (as a
>> - * <tt>const char *</tt>), commit author (as a <tt>const char *</tt>), and
>> - * @a callback_baton as arguments. If @a callback returns an error, that
>> - * error will be returned from @c close_edit, otherwise if there was a
>> - * post-commit hook failure, then that error will be returned and will
>> - * have code SVN_ERR_REPOS_POST_COMMIT_HOOK_FAILED.
>> + * Calling @a (*editor)->close_edit completes the commit.
>> *
>> + * If @a callback is non-NULL, then before @c close_edit returns (but
>> + * after the commit has succeeded) @c close_edit will invoke
>> + * @a callback with a filled-in @c svn_commit_info_t *, @a callback_baton,
>> + * and @a pool or some subpool thereof as arguments. If @a callback
>> + * returns an error, that error will be returned from @c close_edit,
>> + * otherwise if there was a post-commit hook failure, then that error
>> + * will be returned with code SVN_ERR_REPOS_POST_COMMIT_HOOK_FAILED.
>> + *
>> * Calling @a (*editor)->abort_edit aborts the commit, and will also
>> * abort the commit transaction unless @a txn was supplied (not @c
>> * NULL). Callers who supply their own transactions are responsible
>> Index: subversion/libsvn_repos/commit.c
>> ===================================================================
>> --- subversion/libsvn_repos/commit.c (revision 31587)
>> +++ subversion/libsvn_repos/commit.c (working copy)
>> @@ -713,7 +713,7 @@
>> new_revision, SVN_PROP_REVISION_AUTHOR,
>> pool);
>>
>> - if (! err2)
>> + if (eb->commit_callback && (! err2))
>> {
>> commit_info = svn_create_commit_info(pool);
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
>> For additional commands, e-mail: dev-help@subversion.tigris.org
>>
>>
>
>
>
> --
> David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: use NULL commit callback in svn_repos_get_commit_editor5?
Posted by David Glasser <gl...@davidglasser.net>.
On Wed, Jun 4, 2008 at 10:24 PM, Karl Fogel <kf...@red-bean.com> wrote:
> Neels Janosch Hofmeyr <ne...@elego.de> writes:
>> I see that it is mandatory to pass a non-NULL commit callback function
>> for svn_repos_get_commit_editor5, since otherwise a
>> editor->close_edit(..) aborts with a segmentation fault :(
>>
>> Writing a C test, I just wanted nothing to happen when the commit is
>> done, but still had to write a dummy commit callback that does nothing
>> to avoid the segmentation fault.
>>
>> Am I not getting something?
>
> I think it would be an improvement to the API to allow the callback to
> be NULL, sure. Does the (completely untested) patch below work?
I dunno, but looking at the original code it looks like there's an
error leak if either svn_fs_revision_prop call fails. And the call to
svn_error_clear(err), as well as the final "return err;", seem wrong,
since err should be guaranteed to be SVN_NO_ERROR by that point. Am I
missing something?
--dave
> [[[
> Improve an API by allowing a callback to be null.
>
> Suggested by: Neels Janosch Hofmeyr <ne...@elego.de>
>
> * subversion/include/svn_repos.h
> (svn_repos_get_commit_editor5): Document that callback can be null.
> Also, fix up some obsolete documentation: the modern callback type
> takes a struct instead of separate parameters.
>
> * subversion/libsvn_repos/commit.c
> (close_edit): Test callback before calling.
> ]]]
>
> Index: subversion/include/svn_repos.h
> ===================================================================
> --- subversion/include/svn_repos.h (revision 31587)
> +++ subversion/include/svn_repos.h (working copy)
> @@ -938,15 +938,16 @@
> * due to authz will return SVN_ERR_AUTHZ_UNREADABLE or
> * SVN_ERR_AUTHZ_UNWRITABLE.
> *
> - * Calling @a (*editor)->close_edit completes the commit. Before
> - * @c close_edit returns, but after the commit has succeeded, it will
> - * invoke @a callback with the new revision number, the commit date (as a
> - * <tt>const char *</tt>), commit author (as a <tt>const char *</tt>), and
> - * @a callback_baton as arguments. If @a callback returns an error, that
> - * error will be returned from @c close_edit, otherwise if there was a
> - * post-commit hook failure, then that error will be returned and will
> - * have code SVN_ERR_REPOS_POST_COMMIT_HOOK_FAILED.
> + * Calling @a (*editor)->close_edit completes the commit.
> *
> + * If @a callback is non-NULL, then before @c close_edit returns (but
> + * after the commit has succeeded) @c close_edit will invoke
> + * @a callback with a filled-in @c svn_commit_info_t *, @a callback_baton,
> + * and @a pool or some subpool thereof as arguments. If @a callback
> + * returns an error, that error will be returned from @c close_edit,
> + * otherwise if there was a post-commit hook failure, then that error
> + * will be returned with code SVN_ERR_REPOS_POST_COMMIT_HOOK_FAILED.
> + *
> * Calling @a (*editor)->abort_edit aborts the commit, and will also
> * abort the commit transaction unless @a txn was supplied (not @c
> * NULL). Callers who supply their own transactions are responsible
> Index: subversion/libsvn_repos/commit.c
> ===================================================================
> --- subversion/libsvn_repos/commit.c (revision 31587)
> +++ subversion/libsvn_repos/commit.c (working copy)
> @@ -713,7 +713,7 @@
> new_revision, SVN_PROP_REVISION_AUTHOR,
> pool);
>
> - if (! err2)
> + if (eb->commit_callback && (! err2))
> {
> commit_info = svn_create_commit_info(pool);
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>
>
--
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: use NULL commit callback in svn_repos_get_commit_editor5?
Posted by Karl Fogel <kf...@red-bean.com>.
Neels Janosch Hofmeyr <ne...@elego.de> writes:
> I see that it is mandatory to pass a non-NULL commit callback function
> for svn_repos_get_commit_editor5, since otherwise a
> editor->close_edit(..) aborts with a segmentation fault :(
>
> Writing a C test, I just wanted nothing to happen when the commit is
> done, but still had to write a dummy commit callback that does nothing
> to avoid the segmentation fault.
>
> Am I not getting something?
I think it would be an improvement to the API to allow the callback to
be NULL, sure. Does the (completely untested) patch below work?
[[[
Improve an API by allowing a callback to be null.
Suggested by: Neels Janosch Hofmeyr <ne...@elego.de>
* subversion/include/svn_repos.h
(svn_repos_get_commit_editor5): Document that callback can be null.
Also, fix up some obsolete documentation: the modern callback type
takes a struct instead of separate parameters.
* subversion/libsvn_repos/commit.c
(close_edit): Test callback before calling.
]]]
Index: subversion/include/svn_repos.h
===================================================================
--- subversion/include/svn_repos.h (revision 31587)
+++ subversion/include/svn_repos.h (working copy)
@@ -938,15 +938,16 @@
* due to authz will return SVN_ERR_AUTHZ_UNREADABLE or
* SVN_ERR_AUTHZ_UNWRITABLE.
*
- * Calling @a (*editor)->close_edit completes the commit. Before
- * @c close_edit returns, but after the commit has succeeded, it will
- * invoke @a callback with the new revision number, the commit date (as a
- * <tt>const char *</tt>), commit author (as a <tt>const char *</tt>), and
- * @a callback_baton as arguments. If @a callback returns an error, that
- * error will be returned from @c close_edit, otherwise if there was a
- * post-commit hook failure, then that error will be returned and will
- * have code SVN_ERR_REPOS_POST_COMMIT_HOOK_FAILED.
+ * Calling @a (*editor)->close_edit completes the commit.
*
+ * If @a callback is non-NULL, then before @c close_edit returns (but
+ * after the commit has succeeded) @c close_edit will invoke
+ * @a callback with a filled-in @c svn_commit_info_t *, @a callback_baton,
+ * and @a pool or some subpool thereof as arguments. If @a callback
+ * returns an error, that error will be returned from @c close_edit,
+ * otherwise if there was a post-commit hook failure, then that error
+ * will be returned with code SVN_ERR_REPOS_POST_COMMIT_HOOK_FAILED.
+ *
* Calling @a (*editor)->abort_edit aborts the commit, and will also
* abort the commit transaction unless @a txn was supplied (not @c
* NULL). Callers who supply their own transactions are responsible
Index: subversion/libsvn_repos/commit.c
===================================================================
--- subversion/libsvn_repos/commit.c (revision 31587)
+++ subversion/libsvn_repos/commit.c (working copy)
@@ -713,7 +713,7 @@
new_revision, SVN_PROP_REVISION_AUTHOR,
pool);
- if (! err2)
+ if (eb->commit_callback && (! err2))
{
commit_info = svn_create_commit_info(pool);
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org