You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Ivan Zhakov <iv...@visualsvn.com> on 2009/02/27 22:04:28 UTC
Re: svn commit: r36202 - trunk/subversion/libsvn_ra_serf
On Sat, Feb 28, 2009 at 12:50 AM, Bert Huijben <rh...@sharpsvn.net> wrote:
> @@ -280,7 +281,7 @@ handle_checkout(serf_request_t *request,
> }
> apr_uri_parse(pool, location, &uri);
>
> - ctx->resource_url = apr_pstrdup(ctx->pool, uri.path);
> + ctx->resource_url = svn_uri_canonicalize(uri.path, ctx->pool);
> }
You introduce memory management error here, because
svn_uri_canonicalize() does not guarantee that passed uri will be
copied to pool. See include\svn_direent_uri.h:323
[[[
* The returned uri may be statically allocated, equal to @a uri, or
* allocated from @a pool.
]]]
--
Ivan Zhakov
VisualSVN Team
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1241096
Re: svn commit: r36202 - trunk/subversion/libsvn_ra_serf
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Sat, Feb 28, 2009 at 1:16 AM, Bert Huijben <be...@vmoo.com> wrote:
>> -----Original Message-----
>> From: Ivan Zhakov [mailto:ivan@visualsvn.com]
>> Sent: Friday, February 27, 2009 11:04 PM
>> To: dev@subversion.tigris.org; rhuijben@sharpsvn.net
>> Subject: Re: svn commit: r36202 - trunk/subversion/libsvn_ra_serf
>>
>> On Sat, Feb 28, 2009 at 12:50 AM, Bert Huijben <rh...@sharpsvn.net>
>> wrote:
>> > @@ -280,7 +281,7 @@ handle_checkout(serf_request_t *request,
>> > }
>> > apr_uri_parse(pool, location, &uri);
>
> This allocates uri in pool.
>
>> >
>> > - ctx->resource_url = apr_pstrdup(ctx->pool, uri.path);
>> > + ctx->resource_url = svn_uri_canonicalize(uri.path, ctx->pool);
>> > }
>> You introduce memory management error here, because
>> svn_uri_canonicalize() does not guarantee that passed uri will be
>> copied to pool. See include\svn_direent_uri.h:323
>> [[[
>> * The returned uri may be statically allocated, equal to @a uri, or
>> * allocated from @a pool.
>> ]]]
>
> As uri is also allocated in pool, this can't create lifetime issues.
But uri should by allocated in ctx->pool instead of pool passed to
handle_checkout() that's why apr_pstrdup() used before to copy uri to
ctx->pool.
--
Ivan Zhakov
VisualSVN Team
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1241280
RE: svn commit: r36202 - trunk/subversion/libsvn_ra_serf
Posted by Bert Huijben <rh...@sharpsvn.net>.
> -----Original Message-----
> From: Ivan Zhakov [mailto:ivan@visualsvn.com]
> Sent: Friday, February 27, 2009 11:04 PM
> To: dev@subversion.tigris.org; rhuijben@sharpsvn.net
> Subject: Re: svn commit: r36202 - trunk/subversion/libsvn_ra_serf
>
> On Sat, Feb 28, 2009 at 12:50 AM, Bert Huijben <rh...@sharpsvn.net>
> wrote:
> > @@ -280,7 +281,7 @@ handle_checkout(serf_request_t *request,
> > }
> > apr_uri_parse(pool, location, &uri);
This allocates uri in pool.
> >
> > - ctx->resource_url = apr_pstrdup(ctx->pool, uri.path);
> > + ctx->resource_url = svn_uri_canonicalize(uri.path, ctx->pool);
> > }
> You introduce memory management error here, because
> svn_uri_canonicalize() does not guarantee that passed uri will be
> copied to pool. See include\svn_direent_uri.h:323
> [[[
> * The returned uri may be statically allocated, equal to @a uri, or
> * allocated from @a pool.
> ]]]
As uri is also allocated in pool, this can't create lifetime issues.
Bert
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1241136