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:
> > --- 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
> > @@ -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,
> > @@ -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,
> > dst_subdir = svn_dirent_join(dst_fs->path, PATH_REVPROPS_DIR,
> > - 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.
Join us this October at Subversion Live
for two days of best practice SVN training, networking, live demos,
committer meet and greet, and more! Space is limited, so get signed up
Received on 2012-10-18 03:03:38 CEST