[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

RE: Moves in FSFS

From: Bert Huijben <bert_at_qqmail.nl>
Date: Sun, 29 Sep 2013 20:46:17 +0200

                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

This is an archived mail posted to the Subversion Dev mailing list.