[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn_fs_commit_txn and svn_repos_fs_commit_txn inconsistency

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Wed, 22 Dec 2010 02:03:14 +0200

Blair Zajac wrote on Tue, Dec 21, 2010 at 15:55:30 -0800:
> On 12/21/10 3:22 PM, Blair Zajac wrote:
>> On 12/21/10 10:55 AM, Blair Zajac wrote:
>>> On 12/21/10 10:40 AM, Daniel Shahaf wrote:
>>>> Blair Zajac wrote on Tue, Dec 21, 2010 at 10:16:56 -0800:
>>>>> On 12/20/10 11:32 AM, C. Michael Pilato wrote:
>>>>>> On 12/20/2010 02:14 PM, Blair Zajac wrote:
>>>>>>> Shouldn't svn_repos_fs_commit_txn() always run the post-commit hook if
>>>>>>> new_rev is a valid rev?
>>>>>>
>>>>>> That does seem reasonable, yes.
>>>>>
>>>>> Looking through our code, no existing use of svn_fs_commit_txn() and
>>>>> svn_repos_fs_commit_txn() use SVN_IS_VALID_REVNUM(new_rev), the code
>>>>> checks for a non-NULL svn_error_t * and checks if the parent error is a
>>>>> SVN_ERR_REPOS_POST_COMMIT_HOOK_FAILED. It also doesn't scan the chain
>>>>> for that error.
>>>>>
>>>>
>>>> svn_error_has_cause() could be used for that.
>>
>> Daniel,
>>
>> I need a version of svn_error_has_cause() that returns the actual error. How
>> about replacing svn_error_has_cause() with svn_error_find_cause() that returns a
>> NULL or the error? It could still be used for the Boolean test case in this way.
>
> I checked if you were on IRC, but I've got a deadline on a project I'm
> working on and needed this, so I went ahead and made the change.
>
> r1051702
>

I wasn't at the computer then.

Patch looks good. My only comment is that the docstring might want to
warn against using svn_error_clear() on the returned value.

> Blair
Received on 2010-12-22 01:06:13 CET

This is an archived mail posted to the Subversion Dev mailing list.