RE: Moves in FSFS

                Hi Stefan,

Are you sure there are no behavior changes by changing this?

If you copy nodes at every level the resulting nodes are the same as when
you only copy a root. (We had a lot of this in the wc-1.0 code). Tweaking
between copyfrom_* and copyroot_* is quite easy to get not exactly right,
without visible side effects. The repos_layer will hide a lot of those
differences to the client layer, while there can be quite a different
behavior in the storage layer.

With just the snippets in this thread I can't find a true answer and I need
more time to look at the actual change itself, to determine if there are
possible side effects.

                Bert

From: Stefan Fuhrmann [mailto:stefan.fuhrmann_at_wandisco.com]
Sent: zondag 29 september 2013 18:39
To: Bill Tutt
Cc: Branko Čibej; Subversion Development
Subject: Re: Moves in FSFS

On Sun, Sep 29, 2013 at 2:08 AM, Bill Tutt <bill_at_tutts.org
<mailto:bill_at_tutts.org> > wrote:

On a slightly different note I noticed this oddity in lib_fs_fs/dag.c from
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/dag.c?
annotate=1517479

703

jpieper

 
<http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/dag.c
?r1=849538&r2=849539&> 849539

if (is_parent_copyroot)

704

jpieper

 
<http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/dag.c
?r1=849531&r2=849532&> 849532

{

705

danielsh

 
<http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/dag.c
?r1=1305395&r2=1305396&> 1305396

SVN_ERR(get_node_revision(&parent_noderev, parent));

706

jpieper

 
<http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/dag.c
?r1=849538&r2=849539&> 849539

noderev->copyroot_rev = parent_noderev->copyroot_rev;

707

kfogel

 
<http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/dag.c
?r1=858544&r2=858545&> 858545

noderev->copyroot_path = apr_pstrdup(pool,

708

parent_noderev->copyroot_path);

709

jpieper

 
<http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/dag.c
?r1=849531&r2=849532&> 849532

}

710

hwright

 
<http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/dag.c
?r1=866390&r2=866391&> 866391

711

jpieper

 
<http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/dag.c
?r1=849538&r2=849539&> 849539

noderev->copyfrom_path = NULL;

712

noderev->copyfrom_rev = SVN_INVALID_REVNUM;

713

hwright

 
<http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/dag.c
?r1=866390&r2=866391&> 866391

Sure looks like a useless memory allocation from the APR pool for
copyroot_path as well as dead code rotting away quietly.

As well as this earlier in the same file:

394

kfogel

 
<http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/dag.c
?r1=858544&r2=858545&> 858545

new_noderev.copyroot_path = apr_pstrdup(pool,

395

parent_noderev->copyroot_path);

396

jpieper

 
<http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/dag.c
?r1=849538&r2=849539&> 849539

new_noderev.copyroot_rev = parent_noderev->copyroot_rev;

397

new_noderev.copyfrom_rev = SVN_INVALID_REVNUM;

398

new_noderev.copyfrom_path = NULL;

Wow, this has been here for ages... :)

Thanks Bill for spotting this. Fixed in r1527344.

-- Stefan^2.
Received on 2013-09-29 20:47:14 CEST