Re: svn commit: r1330058 - in /subversion/trunk/subversion: include/svn_error_codes.h include/svn_fs.h libsvn_fs/editor.c libsvn_repos/commit.c

On Wed, Apr 25, 2012 at 07:28, Daniel Shahaf <danielsh_at_elego.de> wrote:
>...
>> @@ -326,15 +326,17 @@ complete_cb(void *baton,
>>  {
>>    struct edit_baton *eb = baton;
>>
>> +  /* Watch out for a following call to svn_fs_editor_commit(). Note that
>> +     we are likely here because svn_fs_editor_commit() was called, and it
>> +     invoked svn_editor_complete().  */
>> +  eb->completed = TRUE;
>> +
>
> The code doesn't prevent people from calling svn_editor_complete() twice
> when the second call is not via svn_fs_editor_commit(), but the
> docstring says it would error in that case.

When SVN_DEBUG, svn_editor prevents double-calls to complete(). And
since svn_fs_editor_commit() calls complete(), if a caller followed
with a call to complete, they'd get an editor assertion.

Now. That isn't quite what the docstring describes, but the important
part is "don't call both", and that is prevented when SVN_DEBUG. I
didn't think it important to enforce strictly, and even considered
moving the above "completed" flag into SVN_DEBUG.

Thoughts?

> (Nice handling of all the various branches in svn_fs_editor_commit(),
> BTW.  Tricky code.)

Thanks :-) ... I think the current outputs/definition of
svn_fs_editor_commit() is cleaner than some of the various handling we
have scattered about to deal with some of this stuff.

I appreciate the summary you put into svn_fs.h. I just realized that I
need to further update that docstring: the txn will be committed or
aborted upon exit of the function. Callers don't need to worry about
cleanup.

Cheers,
-g
Received on 2012-04-25 22:16:42 CEST