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

[PATCH] Was: Re: Problem in 1.3.0 RC-6

From: Paul Burba <paulb_at_softlanding.com>
Date: 2005-12-16 21:04:20 CET

Mark Phippard/SoftLanding Systems wrote on 12/16/2005 11:59:58 AM:

> I have encountered a problem in 1.3.0 RC-6. I will explain the
> problem and let you all decide how serious it is. I do not think it
> will pose any problems for the command line, but it may very well
> pose problems for bindings and other clients.
>
> The problem is in the svn status -u output via DAV.
>
> I initially suspected the problem was caused by the change in r17589
> which was backported to 1.3.0 in r17593, because that was a fix for
> a similar problem. My colleague Paul Burba confirmed the problem in
> debug of RC-6 and the command line. However, he has also rebuilt
> RC-4 and confirmed that it has the same problem. So whatever the
> cause of the problem, it appears it has existed in 1.3.0 all along.
>
> Anyway, the problem is that the URL that represents the item in the
> repository and is passed back in the status structure is
> "corrupted". Occasionally it has the right value, most often it has
> an empty string but sometimes it has a partial string or even a
> valid URL of a different item.
>
> This causes problems in Subclipse via the JavaHL bindings. I did
> not spot this before because we only reference the URL when you want
> to do a compare and I did not test that. The change in r17589 was
> fixing a problem where the lastChangedRevision was not correct.
> That is more visible in the UI in Subclipse so I based my testing off
that.
>
> Like I said, I do not think the URL is used by the CLI but it is
> there in the structure so might effect other apps that use it.
>
> Paul Burba is building 1.2.x so that he can see if it had this
> problem or not. I will let him post what he discovers.

Hi All,

I think I found what's wrong and it appears to be a classic pool lifetime
problem. When tweak_statushash copies out of date information from the
file_baton/dir_baton it is operating on, it just assigns the value in the
baton to the svn_wc_status2_t structure. Later, end_element in fetch.c
destroys the baton's pool before svn_client_status2 is complete, so the
ood string is subject to corruption.

The following patch to trunk uses apr_pstrdup to give all ood strings the
necessary(?) lifetimes.

If anyone has some time to review we'd appreciate it.

Thanks,

Paul B.

[[[
Follow-up to r16344 and r16494.

Fix potential corruption of out of date info in svn_wc_status2_t.
 
* subversion/libsvn_wc/status.c
   (tweak_statushash): When copying out of date string info to the
    svn_wc_status2_t structure, use apr_pstrdup rather than assigning
    directly to the file/dir baton since the baton will be destroyed.
]]]

Index: subversion/libsvn_wc/status.c
===================================================================
--- subversion/libsvn_wc/status.c (revision 17694)
+++ subversion/libsvn_wc/status.c (working copy)
@@ -1054,7 +1054,7 @@
     {
       struct dir_baton *b = baton;
       if (b->url)
- statstruct->url = b->url;
+ statstruct->url = apr_pstrdup (pool, b->url);
       statstruct->ood_kind = b->ood_kind;
       /* The last committed rev, date, and author for deleted items
          isn't available. */
@@ -1062,18 +1062,20 @@
         {
           statstruct->ood_last_cmt_rev = b->ood_last_cmt_rev;
           statstruct->ood_last_cmt_date = b->ood_last_cmt_date;
- statstruct->ood_last_cmt_author = b->ood_last_cmt_author;
+ statstruct->ood_last_cmt_author =
+ apr_pstrdup (pool, b->ood_last_cmt_author);
         }
     }
   else
     {
       struct file_baton *b = baton;
       if (b->url)
- statstruct->url = b->url;
+ statstruct->url = apr_pstrdup (pool, b->url);
       statstruct->ood_last_cmt_rev = b->ood_last_cmt_rev;
       statstruct->ood_last_cmt_date = b->ood_last_cmt_date;
       statstruct->ood_kind = b->ood_kind;
- statstruct->ood_last_cmt_author = b->ood_last_cmt_author;
+ statstruct->ood_last_cmt_author = apr_pstrdup (pool,
+ b->ood_last_cmt_author);
     }
   return SVN_NO_ERROR;
 }

_____________________________________________________________________________
Scanned for SoftLanding Systems, Inc. and SoftLanding Europe Plc by IBM Email Security Management Services powered by MessageLabs.
_____________________________________________________________________________

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Dec 16 21:09:04 2005

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