Stefan Sperling wrote on Thu, Sep 08, 2011 at 14:21:34 +0200:
> On Thu, Sep 08, 2011 at 01:39:24AM -0000, [email protected] wrote:
> > Author: danielsh
> > Date: Thu Sep 8 01:39:23 2011
> > New Revision: 1166496
> >
> > URL: http://svn.apache.org/viewvc?rev=1166496&view=rev
> > Log:
> > On the fs-successor-ids branch, factor out repeated logic.
> >
> > This misbehaves a little when I test it, but eyeballing the diff convinces
> > me that those the behaviour of the code must be identical before/after this
> > patch... so I'm committing it anyway :).
>
> How does it misbehave?
>
I chmod'd revs/ shards and files and successors/ shards and files, and
committed revisions ('svn mkdir file://$PWD/r1/$RANDOM -mm' in a loop)
until new shards were opened, and then I examined the permissions on the
new shards. The permissions weren't always preserved, and I think in at
least one case the 0100 bit was removed from a directory.
> > ==============================================================================
> > --- subversion/branches/fs-successor-ids/subversion/libsvn_fs_fs/fs_fs.c
> > (original)
> > +++ subversion/branches/fs-successor-ids/subversion/libsvn_fs_fs/fs_fs.c
> > Thu Sep 8 01:39:23 2011
> > @@ -494,6 +494,32 @@ path_node_origin(svn_fs_t *fs, const cha
> > node_id_minus_last_char, NULL);
> > }
> >
> > +static svn_error_t *
> > +make_shard_dir(const char *(*path_some_shard)(svn_fs_t *,
> > + svn_revnum_t,
> > + apr_pool_t *),
>
> This is awkward.
> Why not pass the shard path as an argument to this function?
>
What would making this change gain?
> > + svn_fs_t *fs,
> > + svn_revnum_t revision,
> > + apr_pool_t *pool)
> > +{
> > + /* We don't care if this fails because the shard already existed
> > + * for some reason. */
> > + svn_error_t *err;
> > + const char *new_dir;
> > +
> > + new_dir = path_some_shard(fs, revision, pool);
> > + err = svn_io_dir_make(new_dir, APR_OS_DEFAULT, pool);
> > + if (err && !APR_STATUS_IS_EEXIST(err->apr_err))
> > + SVN_ERR(err);
> > + svn_error_clear(err);
> > + SVN_ERR(svn_fs_fs__dup_perms(new_dir,
> > + svn_dirent_dirname(new_dir, pool),
> > + pool));
> > +
> > + return SVN_NO_ERROR;
> > +}
