Re: svn commit: rev 1670 - trunk/subversion/libsvn_ra_dav

[Greg Stein <gs...@lyra.org>]

On Thu, Apr 11, 2002 at 01:59:12PM -0500, sussman@tigris.org wrote:
>...
> This fix makes 'svn merge' and 'svn diff' work correctly over DAV now

Yay! Thanks Ben!

>...
> +++ trunk/subversion/libsvn_ra_dav/props.c	Thu Apr 11 13:59:09 2002
>...
>    uri_parse (url, &parsed_url, NULL);
>  
>    /* ### do we want to optimize the props we fetch, based on what the
>       ### user has requested? i.e. omit resourcetype when is_dir is NULL
>       ### and omit relpath when bc_relative is NULL. */
> -  SVN_ERR( svn_ra_dav__get_props_resource(&rsrc, sess, parsed_url.path,
> -                                          NULL, starting_props, pool) );
> +
> +  {
> +    /* Try to get the starting_props from the public url.  If the
> +       resource no longer exists in HEAD, we'll get a failure.  That's
> +       fine: just keep removing components and trying to get the
> +       starting_props from parent directories. */
> +    svn_error_t *err;
> +    svn_stringbuf_t *path_s = svn_stringbuf_create (parsed_url.path, pool);

We should call uri_free() at this point. Otherwise, it is possible to exit
the function without freeing the URI info (a memory leak, particularly if a
caller is ignoring the error and continuing).

[ and it looks like the old code has this problem, too ]

> +
> +    while (! svn_path_is_empty (path_s))
> +      {
> +        err = svn_ra_dav__get_props_resource(&rsrc, sess, path_s->data,
> +                                             NULL, starting_props, pool);
> +        if (! err)
> +          break;   /* found an existing parent! */
> +
> +        if (err->apr_err != SVN_ERR_RA_REQUEST_FAILED)
> +          return err;  /* found a _real_ error */

Here is the leaky return.

> +        /* else... lop off the basename and try again. */
> +        lopped_path = svn_path_join (lopped_path,
> +                                     svn_path_basename (path_s->data, pool),
> +                                     pool);
> +        svn_path_remove_component (path_s);

Spaces before parens!

> +
> +      }
> +
> +    if (svn_path_is_empty (path_s))
> +      /* entire URL was bogus;  not a single part of it exists in
> +         the repository!  */
> +      return svn_error_createf(SVN_ERR_RA_ILLEGAL_URL, 0, NULL, pool,
> +                               "No part of path '%s' was found in"
> +                               "repository HEAD.", parsed_url.path);

And it could be leaky here.

> +  }
> +
>    uri_free(&parsed_url);

Here's the uri_free() to move.

>...
> @@ -519,6 +555,9 @@
>                                    "found on the resource.");
>          }
>  
> +      /* don't forget to tack on the parts we lopped off in order
> +         to find the VCC... */
> +      bc_relative->data = svn_path_join (relative_path, lopped_path, pool);

Space! Ick! :-)

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