On Mon, Nov 23, 2009 at 11:32:51AM +0530, Kannan wrote:
> [[[
> Log:
> Make a proper fix to resolve some deprecation warnings using
> `svn_path_url_add_component2()' by canonicalizing the base before
> passing to the above method; and a minor indentation fix.
>
> [in subversion/libsvn_ra_neon]
>
> * commit.c
> (get_version_url, create_activity, commit_add_dir, commit_add_file
> commit_close_file, add_child): Use `svn_path_url_add_component2()'.
> (commit_delete_entry): Use `svn_path_url_add_component2()' and a minor
> indentation fix in comment.
> (svn_ra_neon__get_commit_editor, checkout_resource): Canonicalize the
> base before passing to `svn_path_url_add_component2()'.
>
> * props.c
> (svn_ra_neon__get_baseline_info): Canonicalize the base before passing
> to `svn_path_url_add_component2()'.
>
> Patch by: Kannan R <[email protected]>
> ]]]
Sorry for the delay.
> Index: ../../subversion/libsvn_ra_neon/commit.c
> ===================================================================
> --- ../../subversion/libsvn_ra_neon/commit.c (revision 882427)
> +++ ../../subversion/libsvn_ra_neon/commit.c (working copy)
> @@ -519,6 +519,12 @@
> }
>
> rsrc->wr_url = apr_pstrdup(rsrc->pool, parse.path);
> +
> + /* Canonicalize the base here as `svn_path_url_add_component2()'
> + won't handle it */
> + rsrc->vsn_url = svn_uri_canonicalize(rsrc->vsn_url, pool);
> + rsrc->wr_url = svn_uri_canonicalize(rsrc->wr_url, pool);
Canonicalising in place looks suspicous. Why do we let these
strings run around in non-canonicalised form for a while, and then
later canonicalise them? There is code above these lines that also
uses rsrc->vsn_url, for instance.
Isn't it possible to canonicalise these strings earlier, when rsrc
is allocated and initialised?
> Index: ../../subversion/libsvn_ra_neon/props.c
> ===================================================================
> --- ../../subversion/libsvn_ra_neon/props.c (revision 882427)
> +++ ../../subversion/libsvn_ra_neon/props.c (working copy)
> @@ -991,7 +991,12 @@
>
> /* maybe return bc_url to the caller */
> if (bc_url)
> - *bc_url = *my_bc_url;
> + {
> + /* Canonicalize the base here as `svn_path_url_add_component2()'
> + won't handle it */
> + bc_url->data = svn_uri_canonicalize(my_bc_url->data, pool);
> + bc_url->len = my_bc_url->len;
Here, my_bc_url() comes from svn_ra_neon__get_baseline_props().
Why doesn't svn_ra_neon__get_baseline_props() canonicalise its result?
Thanks,
Stefan
