On Tue, Oct 16, 2012 at 12:40 PM, Stefan Sperling <stsp_at_elego.de> wrote:
> On Tue, Oct 16, 2012 at 01:12:10AM -0000, stefan2_at_apache.org wrote:
> > Author: stefan2
> > Date: Tue Oct 16 01:12:09 2012
> > New Revision: 1398598
> >
> > URL: http://svn.apache.org/viewvc?rev=1398598&view=rev
> > Log:
> > Fix svnadmin hotcopy for packed format 6 repos. Packed revprops were
> > not copied properly. Fixes issue #4246.
> >
> > * subversion/libsvn_fs_fs/fs_fs.c
> > (hotcopy_copy_packed_shard): use "packed" copying for packed revprops
> >
> > 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=1398598&r1=1398597&r2=1398598&view=diff
> >
> ==============================================================================
> > --- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
> > +++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Tue Oct 16 01:12:09
> 2012
> > @@ -10316,6 +10316,7 @@ hotcopy_copy_packed_shard(svn_revnum_t *
> > const char *src_subdir_packed_shard;
> > svn_revnum_t revprop_rev;
> > apr_pool_t *iterpool;
> > + fs_fs_data_t *src_ffd = src_fs->fsap_data;
> >
> > /* Copy the packed shard. */
> > src_subdir = svn_dirent_join(src_fs->path, PATH_REVS_DIR,
> scratch_pool);
> > @@ -10333,18 +10334,43 @@ hotcopy_copy_packed_shard(svn_revnum_t *
> > /* Copy revprops belonging to revisions in this pack. */
> > src_subdir = svn_dirent_join(src_fs->path, PATH_REVPROPS_DIR,
> scratch_pool);
> > dst_subdir = svn_dirent_join(dst_fs->path, PATH_REVPROPS_DIR,
> scratch_pool);
> > - iterpool = svn_pool_create(scratch_pool);
> > - for (revprop_rev = rev;
> > - revprop_rev < rev + max_files_per_dir;
> > - revprop_rev++)
> > +
> > + if ( src_ffd->format < SVN_FS_FS__MIN_PACKED_REVPROP_FORMAT
> > + || src_ffd->min_unpacked_rev < rev + max_files_per_dir)
>
> The spacing here is a bit... unconventional.
>
I completely agree but I've grown to like it
for the following reasons:
> I'd expect this code to be formatted like this:
>
> if (src_ffd->format < SVN_FS_FS__MIN_PACKED_REVPROP_FORMAT
> || src_ffd->min_unpacked_rev < rev + max_files_per_dir)
>
Operand "nesting" level is obscured (just slightly).
Remember, how you learned in school to do a tabular /
place-aligned addition or subtraction.
> or like this:
>
> if (src_ffd->format < SVN_FS_FS__MIN_PACKED_REVPROP_FORMAT ||
> src_ffd->min_unpacked_rev < rev + max_files_per_dir)
>
That moves the root of the operation tree, i.e. the ||
operation, out of sight.
> Other than that, this commit is looking good to me.
>
I don't cling to my tabular formatting but I use it for
the above reasons rather than at a whim.
-- Stefan^2.
--
*
Join us this October at Subversion Live
2012<http://www.wandisco.com/svn-live-2012>
for two days of best practice SVN training, networking, live demos,
committer meet and greet, and more! Space is limited, so get signed up
today<http://www.wandisco.com/svn-live-2012>
!
*
Received on 2012-10-18 03:03:38 CEST