Julian Foad <julianfoad@btopenworld.com> writes:
> I'm not sure when URLs are supposed to be canonicalised, but there was
> definitely a bug or at least a code redundancy introduced by this
> revision.
>
> > Index: subversion/libsvn_client/checkout.c
> > ===================================================================
> > --- subversion/libsvn_client/checkout.c (revision 12364)
> > +++ subversion/libsvn_client/checkout.c (revision 12365)
> > @@ -42,8 +42,9 @@
> > svn_error_t *
> > svn_client__checkout_internal (svn_revnum_t *result_rev,
> > - const char *URL,
> > + const char *url,
> > const char *path,
> > + const svn_opt_revision_t *peg_revision,
> > const svn_opt_revision_t *revision,
> > svn_boolean_t recurse,
> > svn_boolean_t *timestamp_sleep,
> > @@ -55,6 +56,7 @@
> > svn_revnum_t revnum;
> > svn_boolean_t sleep_here = FALSE;
> > svn_boolean_t *use_sleep = timestamp_sleep ? timestamp_sleep : &sleep_here;
> > + const char *URL;
> > /* Sanity check. Without these, the checkout is meaningless. */
> > assert (path != NULL);
> > @@ -67,27 +69,19 @@
> > return svn_error_create (SVN_ERR_CLIENT_BAD_REVISION, NULL, NULL);
> > /* Canonicalize the URL. */
> > - URL = svn_path_canonicalize (URL, pool);
> > + URL = svn_path_canonicalize (url, pool);
>
> This canonicalisation is now lost ...
>
> > {
> > - void *ra_baton, *session;
> > + void *session;
> > svn_ra_plugin_t *ra_lib;
> > svn_node_kind_t kind;
> > const char *uuid;
> > - /* Get the RA vtable that matches URL. */
> > - SVN_ERR (svn_ra_init_ra_libs (&ra_baton, pool));
> > - SVN_ERR (svn_ra_get_ra_library (&ra_lib, ra_baton, URL, pool));
> > -
> > - /* Open an RA session to URL. Note that we do not have an admin area
> > - for storing temp files. */
> > - SVN_ERR (svn_client__open_ra_session (&session, ra_lib, URL, NULL,
> > - NULL, NULL, FALSE, TRUE,
> > - ctx, pool));
> > -
> > - SVN_ERR (svn_client__get_revision_number
> > - (&revnum, ra_lib, session, revision, path, pool));
> > -
> > + /* Get the RA connection. */
> > + SVN_ERR (svn_client__ra_lib_from_path (&ra_lib, &session, &revnum,
> > + &URL, url, peg_revision,
>
> ... because here you're using "url" as input, and overwriting the
> canonicalised "URL" with a new value without using it.
>
> So, is that canonicalisation still wanted?
And may I add that having two variables named "URL" and "url" in the
same function is just... well, let's say "bad" and leave it at that.
Furthermore, the indentation of that entire curly-delimited block is
wrong anyway.
I have nobly resisted running 'svn blame' :-). I'm afraid of what I
might find. Instead, I've fixed the indentation issue in r16149, and
the URL/url issue, along with the canonicalization problem, in r16150.
-Karl
--
www.collab.net <> CollabNet | Distributed Development On Demand
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Sep 17 01:44:51 2005