You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Blair Zajac <bl...@orcaware.com> on 2011/07/09 00:50:17 UTC

Re: svn commit: r1144530 - /subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c

On Jul 8, 2011, at 3:44 PM, danielsh@apache.org wrote:

> Author: danielsh
> Date: Fri Jul  8 22:44:52 2011
> New Revision: 1144530
> 
> URL: http://svn.apache.org/viewvc?rev=1144530&view=rev
> Log:
> Repeat r1143899 elsewhere in the same function.
> 
> * subversion/libsvn_fs_fs/fs_fs.c
>  (commit_body): Creating a revprops shard has always been orthogonal
>    of packing, since we have not implemented packing of non-completed
>    shards or just-committed revisions.  Accordingly, replace an if() with
>    an assert().
> 
> Modified:
>    subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
> 
> Modified: subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c?rev=1144530&r1=1144529&r2=1144530&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
> +++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Fri Jul  8 22:44:52 2011
> @@ -6509,8 +6509,7 @@ commit_body(void *baton, apr_pool_t *poo
>                                     new_dir, pool));
>         }
> 
> -      if (ffd->format < SVN_FS_FS__MIN_PACKED_REVPROP_FORMAT ||
> -          new_rev >= ffd->min_unpacked_revprop)
> +      assert(! is_packed_revprop(cb->fs, new_rev));

Wouldn't an SVN_ERR_ASSERT() be better here?

Blair


Re: svn commit: r1144530 - /subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Blair Zajac wrote on Fri, Jul 08, 2011 at 17:52:26 -0700:
> On 7/8/11 4:34 PM, Daniel Shahaf wrote:
> >Blair Zajac wrote on Fri, Jul 08, 2011 at 15:50:17 -0700:
> >>On Jul 8, 2011, at 3:44 PM, danielsh@apache.org wrote:
> >>>+++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Fri Jul  8 22:44:52 2011
> >>>@@ -6509,8 +6509,7 @@ commit_body(void *baton, apr_pool_t *poo
> >>>                                     new_dir, pool));
> >>>         }
> >>>
> >>>-      if (ffd->format<  SVN_FS_FS__MIN_PACKED_REVPROP_FORMAT ||
> >>>-          new_rev>= ffd->min_unpacked_revprop)
> >>>+      assert(! is_packed_revprop(cb->fs, new_rev));
> >>
> >>Wouldn't an SVN_ERR_ASSERT() be better here?
> >
> >Works for me.  What about those other instances --- in
> >write_change_entry() and recover_find_max_ids()?
> 
> +1.
> 

r1144563.

> Blair

Re: svn commit: r1144530 - /subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c

Posted by Blair Zajac <bl...@orcaware.com>.
On 7/8/11 4:34 PM, Daniel Shahaf wrote:
> Blair Zajac wrote on Fri, Jul 08, 2011 at 15:50:17 -0700:
>> On Jul 8, 2011, at 3:44 PM, danielsh@apache.org wrote:
>>> +++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Fri Jul  8 22:44:52 2011
>>> @@ -6509,8 +6509,7 @@ commit_body(void *baton, apr_pool_t *poo
>>>                                      new_dir, pool));
>>>          }
>>>
>>> -      if (ffd->format<  SVN_FS_FS__MIN_PACKED_REVPROP_FORMAT ||
>>> -          new_rev>= ffd->min_unpacked_revprop)
>>> +      assert(! is_packed_revprop(cb->fs, new_rev));
>>
>> Wouldn't an SVN_ERR_ASSERT() be better here?
>
> Works for me.  What about those other instances --- in
> write_change_entry() and recover_find_max_ids()?

+1.

Blair

Re: svn commit: r1144530 - /subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Blair Zajac wrote on Fri, Jul 08, 2011 at 15:50:17 -0700:
> On Jul 8, 2011, at 3:44 PM, danielsh@apache.org wrote:
> > +++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Fri Jul  8 22:44:52 2011
> > @@ -6509,8 +6509,7 @@ commit_body(void *baton, apr_pool_t *poo
> >                                     new_dir, pool));
> >         }
> > 
> > -      if (ffd->format < SVN_FS_FS__MIN_PACKED_REVPROP_FORMAT ||
> > -          new_rev >= ffd->min_unpacked_revprop)
> > +      assert(! is_packed_revprop(cb->fs, new_rev));
> 
> Wouldn't an SVN_ERR_ASSERT() be better here?

Works for me.  What about those other instances --- in
write_change_entry() and recover_find_max_ids()?

Index: subversion/libsvn_fs_fs/fs_fs.c
===================================================================
--- subversion/libsvn_fs_fs/fs_fs.c	(revision 1144550)
+++ subversion/libsvn_fs_fs/fs_fs.c	(working copy)
@@ -5330,7 +5330,7 @@ write_change_entry(apr_file_t *file,
 
   if (include_node_kind)
     {
-      assert(change->node_kind == svn_node_dir
+      SVN_ERR_ASSERT(change->node_kind == svn_node_dir
                      || change->node_kind == svn_node_file);
       kind_string = apr_psprintf(pool, "-%s",
                                  change->node_kind == svn_node_dir
@@ -6371,7 +6371,7 @@ commit_body(void *baton, apr_pool_t *pool)
         }
 
       /* Create the revprops shard. */
-      assert(! is_packed_revprop(cb->fs, new_rev));
+      SVN_ERR_ASSERT(! is_packed_revprop(cb->fs, new_rev));
         {
           const char *new_dir = path_revprops_shard(cb->fs, new_rev, pool);
           svn_error_t *err = svn_io_dir_make(new_dir, APR_OS_DEFAULT, pool);
@@ -6407,7 +6407,7 @@ commit_body(void *baton, apr_pool_t *pool)
                                      &date, pool));
 
   /* Move the revprops file into place. */
-  assert(! is_packed_revprop(cb->fs, new_rev));
+  SVN_ERR_ASSERT(! is_packed_revprop(cb->fs, new_rev));
   revprop_filename = path_txn_props(cb->fs, cb->txn->id, pool);
   final_revprop = path_revprops(cb->fs, new_rev, pool);
   SVN_ERR(move_into_place(revprop_filename, final_revprop,
@@ -6881,12 +6881,12 @@ recover_find_max_ids(svn_fs_t *fs, svn_revnum_t re
 
       if (svn_fs_fs__key_compare(node_id, max_node_id) > 0)
         {
-          assert(strlen(node_id) < MAX_KEY_SIZE);
+          SVN_ERR_ASSERT(strlen(node_id) < MAX_KEY_SIZE);
           apr_cpystrn(max_node_id, node_id, MAX_KEY_SIZE);
         }
       if (svn_fs_fs__key_compare(copy_id, max_copy_id) > 0)
         {
-          assert(strlen(copy_id) < MAX_KEY_SIZE);
+          SVN_ERR_ASSERT(strlen(copy_id) < MAX_KEY_SIZE);
           apr_cpystrn(max_copy_id, copy_id, MAX_KEY_SIZE);
         }