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

Re: svn_error_purge_tracing() and clearing the errors Re: svn commit: r1051763 - in /subversion/trunk/subversion: include/private/svn_repos_private.h libsvn_repos/commit.c mod_dav_svn/version.c

From: Blair Zajac <blair_at_orcaware.com>
Date: Thu, 23 Dec 2010 17:09:31 -0800

On 12/23/10 3:01 AM, Daniel Shahaf wrote:
> I've made a quick sketch --- looks good?

The suggestions look good.

We could add some more tests, checking the behavior with SVN_NO_ERROR input and
one where all errors are tracing, which should return an error from
SVN_ERR_ASSERT().

As an aside, there's a comment about adding an svn_boolean_t to svn_error_t
instead of using strcmp(), which could be done in a totally separate commit.

>> Index: subversion/mod_dav_svn/deadprops.c
>> ===================================================================
>> --- subversion/mod_dav_svn/deadprops.c (revision 1052167)
>> +++ subversion/mod_dav_svn/deadprops.c (working copy)
>> @@ -222,18 +222,22 @@
>> + purged_serr->message = apr_xml_quote_string
>> + (purged_serr->pool,
>
> Our current policy is to have the '(' on the same line as the function name.

I looked through the hacking guide and didn't see anything, but doing some greps
through our code shows a disposition to this style:

$ cd subversion/
$ grep '^ *(' */*.c| wc -l
1757
$ grep '($' */*.c| wc -l
454

>> Index: subversion/mod_dav_svn/version.c
>> ===================================================================
>> --- subversion/mod_dav_svn/version.c (revision 1052167)
>> +++ subversion/mod_dav_svn/version.c (working copy)
>> @@ -922,16 +922,16 @@
>> {
>> if (serr)
>> {
>> - const char *post_commit_err;
>> - apr_err = serr->apr_err;
>> - post_commit_err = svn_repos__post_commit_error_str
>> - (serr, resource->pool);
>> - serr = SVN_NO_ERROR;
>> - ap_log_perror(APLOG_MARK, APLOG_ERR, apr_err, resource->pool,
>> + const char *post_commit_err = svn_repos__post_commit_error_str
>> + (serr, resource->pool);
>> + ap_log_perror(APLOG_MARK, APLOG_ERR, serr->apr_err,
>> + resource->pool,
>
> The rest looks good. (I am assuming --- didn't check --- that you
> updated all callers.)

Yes.

Blair
Received on 2010-12-24 02:11:04 CET

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.