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

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