You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Sander Striker <st...@apache.org> on 2003/03/10 13:54:42 UTC
RE: svn commit: rev 5259 - trunk/subversion/libsvn_ra_dav
> From: cmpilato@tigris.org [mailto:cmpilato@tigris.org]
> Sent: Monday, March 10, 2003 2:23 PM
> Author: cmpilato
> Date: Mon Mar 10 07:22:27 2003
> New Revision: 5259
>
> Modified:
> trunk/subversion/libsvn_ra_dav/commit.c
> Log:
> * subversion/libsvn_ra_dav/commit.c
> (add_valid_target): Ensure proper lifetime of the paths in the hash.
>
> Modified: trunk/subversion/libsvn_ra_dav/commit.c
> ==============================================================================
> --- trunk/subversion/libsvn_ra_dav/commit.c (original)
> +++ trunk/subversion/libsvn_ra_dav/commit.c Mon Mar 10 07:22:27 2003
> @@ -661,7 +661,8 @@
> enum svn_recurse_kind kind)
> {
> apr_hash_t *hash = cc->valid_targets;
> - apr_hash_set (hash, path, APR_HASH_KEY_STRING, &kind);
> + svn_string_t *path_str = svn_string_create(path, apr_hash_pool_get(hash));
> + apr_hash_set (hash, path_str->data, path_str->len, &kind);
> }
Why not just like so?
apr_hash_set (hash, apr_pstrdup (apr_hash_pool_get (hash), path),
APR_HASH_KEY_STRING, &kind);
Sander
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 5259 - trunk/subversion/libsvn_ra_dav
Posted by Branko Čibej <br...@xbc.nu>.
Greg Stein wrote:
>/me looks around for who wrote this stuff... bleh...
>
>
Should I start on "svn blame^H^H^H^H^Hpraise"? :-)
--
Brane Čibej <br...@xbc.nu> http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 5259 - trunk/subversion/libsvn_ra_dav
Posted by cm...@collab.net.
Greg Stein <gs...@lyra.org> writes:
> > Why not just like so?
> >
> > apr_hash_set (hash, apr_pstrdup (apr_hash_pool_get (hash), path),
> > APR_HASH_KEY_STRING, &kind);
>
> Right. That approach saves an svn_string_t from the heap. It does a bit of
> extra length counting, but if that is a problem:
Right. I used that approach to save a strlen()...
> apr_size_t len = strlen(path);
>
> path = apr_pstrmemdup(apr_hash_pool_get(hash), path, len);
> apr_hash_set(hash, path, len, &kind);
...because I forgot about apr_pstrmemdup(). :-(
> Woah!! This whole valid_targets thing is way busted, and has been for a long
> while. Dang... we've just been lucky. First, we had bad path lifetimes,
> which Mike fixed. But we *still* have problems with that kind.
Yah, I don't think it even gets used anymore. My recollection is that
it was stuff for the MERGE response handling, which is currently
disabled. I just happened to notice the pool problem while doing
other things.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 5259 - trunk/subversion/libsvn_ra_dav
Posted by Greg Stein <gs...@lyra.org>.
On Mon, Mar 10, 2003 at 02:54:42PM +0100, Sander Striker wrote:
>...
> > +++ trunk/subversion/libsvn_ra_dav/commit.c Mon Mar 10 07:22:27 2003
> > @@ -661,7 +661,8 @@
> > enum svn_recurse_kind kind)
> > {
> > apr_hash_t *hash = cc->valid_targets;
> > - apr_hash_set (hash, path, APR_HASH_KEY_STRING, &kind);
> > + svn_string_t *path_str = svn_string_create(path, apr_hash_pool_get(hash));
> > + apr_hash_set (hash, path_str->data, path_str->len, &kind);
> > }
>
> Why not just like so?
>
> apr_hash_set (hash, apr_pstrdup (apr_hash_pool_get (hash), path),
> APR_HASH_KEY_STRING, &kind);
Right. That approach saves an svn_string_t from the heap. It does a bit of
extra length counting, but if that is a problem:
apr_size_t len = strlen(path);
path = apr_pstrmemdup(apr_hash_pool_get(hash), path, len);
apr_hash_set(hash, path, len, &kind);
Woah!! This whole valid_targets thing is way busted, and has been for a long
while. Dang... we've just been lucky. First, we had bad path lifetimes,
which Mike fixed. But we *still* have problems with that kind.
We shove the kind in as a pointer to the stack argument. Whoops!
We pull it *out* as an integer (see merge.c::okay_to_bump_path). Eek.
/me looks around for who wrote this stuff... bleh...
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org