You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Hyrum K Wright <hy...@hyrumwright.org> on 2011/04/25 14:28:35 UTC
Re: svn commit: r1096460 - /subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c
This looks like it could be long-standing bug. Backport?
-Hyrum
On Mon, Apr 25, 2011 at 6:47 AM, <rh...@apache.org> wrote:
> Author: rhuijben
> Date: Mon Apr 25 11:47:42 2011
> New Revision: 1096460
>
> URL: http://svn.apache.org/viewvc?rev=1096460&view=rev
> Log:
> For ra_local, allocate the result for svn_ra_get_repos_root() and
> svn_ra_get_uuid() in the right pool. (see svn_ra.h)
>
> Found by testing with apr_allocator_max_free_set(allocator, 1); in
> subversion/svn/main.c.
>
> * subversion/libsvn_ra_local/ra_plugin.c
> (svn_ra_local__get_uuid,
> svn_ra_local__get_repos_root): Duplicate paths in the right pool.
>
> Modified:
> subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c
>
> Modified: subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c?rev=1096460&r1=1096459&r2=1096460&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c (original)
> +++ subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c Mon Apr 25 11:47:42 2011
> @@ -586,7 +586,7 @@ svn_ra_local__get_uuid(svn_ra_session_t
> apr_pool_t *pool)
> {
> svn_ra_local__session_baton_t *sess = session->priv;
> - *uuid = sess->uuid;
> + *uuid = apr_pstrdup(pool, sess->uuid);
> return SVN_NO_ERROR;
> }
>
> @@ -596,7 +596,7 @@ svn_ra_local__get_repos_root(svn_ra_sess
> apr_pool_t *pool)
> {
> svn_ra_local__session_baton_t *sess = session->priv;
> - *url = sess->repos_url;
> + *url = apr_pstrdup(pool, sess->repos_url);
> return SVN_NO_ERROR;
> }
>
>
>
>
Re: svn commit: r1096460 - /subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c
Posted by "C. Michael Pilato" <cm...@collab.net>.
I typed the following before realizing that this commit was ultimately
reverted, but figured I'd provide the insight for others' sake anyway.
Bert, you can ignore this. :-)
{{{
Actually, I don't think this is a longstanding bug at all. The
svn_ra_get_uuid() and svn_ra_get_repos_root() interfaces promise to return
these values allocated *in the session pool*. As such, the ra_local
implementations thereof purposely do *not* dup into the passed-in POOL.
It's the svn_ra_get_uuid2() and svn_ra_get_repos_root2() wrapper functions
which do that duplication. Bert's commit needs to be reverted, and the
appropriate pool lifetime fix made elsewhere.
}}}
On 04/25/2011 08:28 AM, Hyrum K Wright wrote:
> This looks like it could be long-standing bug. Backport?
>
> -Hyrum
>
> On Mon, Apr 25, 2011 at 6:47 AM, <rh...@apache.org> wrote:
>> Author: rhuijben
>> Date: Mon Apr 25 11:47:42 2011
>> New Revision: 1096460
>>
>> URL: http://svn.apache.org/viewvc?rev=1096460&view=rev
>> Log:
>> For ra_local, allocate the result for svn_ra_get_repos_root() and
>> svn_ra_get_uuid() in the right pool. (see svn_ra.h)
>>
>> Found by testing with apr_allocator_max_free_set(allocator, 1); in
>> subversion/svn/main.c.
>>
>> * subversion/libsvn_ra_local/ra_plugin.c
>> (svn_ra_local__get_uuid,
>> svn_ra_local__get_repos_root): Duplicate paths in the right pool.
>>
>> Modified:
>> subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c
>>
>> Modified: subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c
>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c?rev=1096460&r1=1096459&r2=1096460&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c (original)
>> +++ subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c Mon Apr 25 11:47:42 2011
>> @@ -586,7 +586,7 @@ svn_ra_local__get_uuid(svn_ra_session_t
>> apr_pool_t *pool)
>> {
>> svn_ra_local__session_baton_t *sess = session->priv;
>> - *uuid = sess->uuid;
>> + *uuid = apr_pstrdup(pool, sess->uuid);
>> return SVN_NO_ERROR;
>> }
>>
>> @@ -596,7 +596,7 @@ svn_ra_local__get_repos_root(svn_ra_sess
>> apr_pool_t *pool)
>> {
>> svn_ra_local__session_baton_t *sess = session->priv;
>> - *url = sess->repos_url;
>> + *url = apr_pstrdup(pool, sess->repos_url);
>> return SVN_NO_ERROR;
>> }
>>
>>
>>
>>
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand