-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Stefan Sperling wrote:
[..]
> 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?
Thank you for the comments, Stefan. Canonicalization of the base is
now done, where they're getting initialized. rsrc->wr_url is
initialized in the same place, but in place canonicalization has been
changed.
>> 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?
`my_bc_rel' gets its value from `svn_ra_neon__get_baseline_props()'
whereas `my_bc_url' is initialized in the same place.
Attaching the updated patch herewith.
[[[
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, apply_revprops):
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]>
]]]
- --
Thanks & Regards,
Kannan
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iQEVAwUBSxPdi3lTqcY7ytmIAQL4FQf7BAFuQz9brjUtGNFKWb/PIcz3srbfsbPh
p50XX91GLSEBjy4PKdQwTJyuDdyOT3UFc0WFSjmy8hFcoAANvyAEWuqOWU+2F4Qk
BQs+n8XLTHfmAT/W3XV5pyUs8eJzQRTAd/Il2Zux3V6gAXihuNEUDshO+1rMBupi
dx8/H5kbmViOB/3/1JaAEXpU5rjM+rKrjylKTPDgqMyeFOahVLcUikDYykgC+pMi
uxgZERdQ+iaYuefxcTBWXMyUIpYFVCSVrQjse28ASmewlWkSm0fZ8Ruzt1cRRmni
/z3AUbRtuGFMvVwXYJ+tGY1OmV7uDl/awVS3x2VufF00xcgHjCvmpg==
=C2dF
-----END PGP SIGNATURE-----
Index: subversion/libsvn_ra_neon/commit.c
===================================================================
--- subversion/libsvn_ra_neon/commit.c (revision 885339)
+++ subversion/libsvn_ra_neon/commit.c (working copy)
@@ -203,9 +203,9 @@
the version resource URL of RSRC. */
if (parent && parent->vsn_url && parent->revision == rsrc->revision)
{
- rsrc->vsn_url = svn_path_url_add_component(parent->vsn_url,
- rsrc->name,
- rsrc->pool);
+ rsrc->vsn_url = svn_path_url_add_component2(parent->vsn_url,
+ rsrc->name,
+ rsrc->pool);
return SVN_NO_ERROR;
}
@@ -231,7 +231,7 @@
rsrc->revision,
pool));
- url = svn_path_url_add_component(bc_url.data, bc_relative.data, pool);
+ url = svn_path_url_add_component2(bc_url.data, bc_relative.data, pool);
}
/* Get the DAV:checked-in property, which contains the URL of the
@@ -325,8 +325,8 @@
the activity, and create the activity. The URL for our activity
will be ACTIVITY_COLL/UUID */
SVN_ERR(get_activity_collection(cc, &activity_collection, FALSE, pool));
- url = svn_path_url_add_component(activity_collection->data,
- uuid_buf, pool);
+ url = svn_path_url_add_component2(activity_collection->data,
+ uuid_buf, pool);
SVN_ERR(svn_ra_neon__simple_request(&code, cc->ras,
"MKACTIVITY", url, NULL, NULL,
201 /* Created */,
@@ -338,8 +338,8 @@
if (code == 404)
{
SVN_ERR(get_activity_collection(cc, &activity_collection, TRUE, pool));
- url = svn_path_url_add_component(activity_collection->data,
- uuid_buf, pool);
+ url = svn_path_url_add_component2(activity_collection->data,
+ uuid_buf, pool);
SVN_ERR(svn_ra_neon__simple_request(&code, cc->ras,
"MKACTIVITY", url, NULL, NULL,
201, 0, pool));
@@ -373,7 +373,7 @@
rsrc->pool = pool;
rsrc->revision = revision;
rsrc->name = name;
- rsrc->url = svn_path_url_add_component(parent->url, name, pool);
+ rsrc->url = svn_path_url_add_component2(parent->url, name, pool);
rsrc->local_path = svn_path_join(parent->local_path, name, pool);
/* Case 1: the resource is truly "new". Either it was added as a
@@ -382,7 +382,7 @@
URL by the rules of deltaV: "copy structure is preserved below
the WR you COPY to." */
if (created || (parent->vsn_url == NULL))
- rsrc->wr_url = svn_path_url_add_component(parent->wr_url, name, pool);
+ rsrc->wr_url = svn_path_url_add_component2(parent->wr_url, name, pool);
/* Case 2: the resource is already under version-control somewhere.
This means it has a VR URL already, and the WR URL won't exist
@@ -518,7 +518,11 @@
_("Unable to parse URL '%s'"), locn);
}
- rsrc->wr_url = apr_pstrdup(rsrc->pool, parse.path);
+ /* Canonicalize the base here as `svn_path_url_add_component2()'
+ won't handle it */
+ rsrc->wr_url = svn_uri_canonicalize
+ (apr_pstrdup(rsrc->pool, parse.path), pool);
+
ne_uri_free(&parse);
return SVN_NO_ERROR;
@@ -714,7 +718,7 @@
SVN_ERR(checkout_resource(parent->cc, parent->rsrc, TRUE, NULL, pool));
/* create the URL for the child resource */
- child = svn_path_url_add_component(parent->rsrc->wr_url, name, pool);
+ child = svn_path_url_add_component2(parent->rsrc->wr_url, name, pool);
/* Start out assuming that we're deleting a file; try to lookup the
path itself in the token-hash, and if found, attach it to the If:
@@ -729,8 +733,8 @@
const char *token_header_val;
const char *token_uri;
- token_uri = svn_path_url_add_component(parent->cc->ras->url->data,
- path, pool);
+ token_uri = svn_path_url_add_component2(parent->cc->ras->url->data,
+ path, pool);
token_header_val = apr_psprintf(pool, "<%s> (<%s>)",
token_uri, token);
extra_headers = apr_hash_make(pool);
@@ -773,7 +777,7 @@
Note that we're not sending the locks in the If: header, for
the same reason we're not sending in MERGE's headers: httpd has
- limits on the amount of data it's willing to receive in headers. */
+ limits on the amount of data it's willing to receive in headers. */
apr_hash_t *child_tokens = NULL;
svn_ra_neon__request_t *request;
@@ -888,9 +892,9 @@
"source" argument to the COPY request. The "Destination:"
header given to COPY is simply the wr_url that is already
part of the child object. */
- copy_src = svn_path_url_add_component(bc_url.data,
- bc_relative.data,
- workpool);
+ copy_src = svn_path_url_add_component2(bc_url.data,
+ bc_relative.data,
+ workpool);
/* Have neon do the COPY. */
SVN_ERR(svn_ra_neon__copy(parent->cc->ras,
@@ -1088,9 +1092,9 @@
"source" argument to the COPY request. The "Destination:"
header given to COPY is simply the wr_url that is already
part of the file_baton. */
- copy_src = svn_path_url_add_component(bc_url.data,
- bc_relative.data,
- workpool);
+ copy_src = svn_path_url_add_component2(bc_url.data,
+ bc_relative.data,
+ workpool);
/* Have neon do the COPY. */
SVN_ERR(svn_ra_neon__copy(parent->cc->ras,
@@ -1271,9 +1275,9 @@
svn_ra_neon__set_header
(extra_headers, "If",
apr_psprintf(pool, "<%s> (<%s>)",
- svn_path_url_add_component(cc->ras->url->data,
- file->rsrc->url,
- request->pool),
+ svn_path_url_add_component2(cc->ras->url->data,
+ file->rsrc->url,
+ request->pool),
file->token));
if (pb->base_checksum)
@@ -1389,8 +1393,11 @@
vcc, NULL,
&svn_ra_neon__checked_in_prop, pool));
baseline_rsrc.pool = pool;
- baseline_rsrc.vsn_url = baseline_url->data;
+ /* Canonicalize the base here as `svn_path_url_add_component2()'
+ won't handle it. */
+ baseline_rsrc.vsn_url = svn_uri_canonicalize(baseline_url->data, pool);
+
/* To set the revision properties, we must checkout the latest baseline
and get back a mutable "working" baseline. */
err = checkout_resource(cc, &baseline_rsrc, FALSE, NULL, pool);
@@ -1452,6 +1459,10 @@
/* ### should we perform an OPTIONS to validate the server we're about
### to talk to? */
+ /* Canonicalize the base here as `svn_path_url_add_component2()'
+ won't handle it */
+ cc->ras->act_coll = svn_uri_canonicalize(cc->ras->act_coll, pool);
+
/*
** Create an Activity. This corresponds directly to an FS transaction.
** We will check out all further resources within the context of this
Index: subversion/libsvn_ra_neon/props.c
===================================================================
--- subversion/libsvn_ra_neon/props.c (revision 885339)
+++ 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;
+ }
if (latest_rev != NULL)
{
