You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@lyra.org> on 2003/06/20 01:14:14 UTC

Re: svn commit: rev 6302 - in trunk/subversion: libsvn_client libsvn_delta libsvn_repos libsvn_wc

On Thu, Jun 19, 2003 at 07:11:09PM -0500, cmpilato@tigris.org wrote:
>...
> +++ trunk/subversion/libsvn_client/commit.c	Thu Jun 19 19:11:07 2003
> @@ -951,19 +951,23 @@
>            svn_wc_adm_access_t *adm_access;
>            const svn_wc_entry_t *entry;
>  
> +          /* Clear the subpool here because there are some 'continue'
> +             statements in this loop. */
> +          svn_pool_clear (subpool);

Note that HACKING says this is now the "standard" pattern, rather than
simply for loops with 'continue' in them.

>...
> +++ trunk/subversion/libsvn_delta/path_driver.c	Thu Jun 19 19:11:07 2003
>...
> @@ -210,10 +214,10 @@
>  
>        /*** Step C - Open any directories between the common ancestor
>             and the parent of the current path. ***/
> -      svn_path_split (path, &pdir, &bname, pool);
> +      svn_path_split (path, &pdir, &bname, iterpool);
>        if (strlen (pdir) > common_len)
>          {
> -          char *rel = apr_pstrdup (pool, pdir);
> +          char *rel = (char *)pdir;
>            char *piece = rel + common_len + 1;

Hmm. How about if we lose 'rel' and just use 'pdir' directly. And cast it
when setting up 'piece'.

I think it is probably important to clarify what is occurring here. We get
back something that is labelled as 'const char *', but we know it is a copy
and that we can change it.

Personally, I think it is a rather poor assumption to make aobut the
internals of svn_path_split(), but hey... I won't get too fussed about it.
But if we *do*, then it ought to be documented.

>...
> +++ trunk/subversion/libsvn_repos/commit.c	Thu Jun 19 19:11:07 2003
>...
> @@ -399,7 +399,7 @@
>    /* Build a new file baton */
>    new_fb = apr_pcalloc (pool, sizeof (*new_fb));
>    new_fb->edit_baton = eb;
> -  new_fb->pool = pool;
> +  new_fb->pool = pool; /* unused? */

Did you try removing it from the structure and compiling? That will show its
usage real fast :-)

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