jpieper@tigris.org writes:
> Log:
> After renaming 'URL' to 'url' in the parameter list of
> svn_client__checkout_internal (r12365), the assertion testing for its
> non-NULLness should be adjusted as well.
>
> * subversion/libsvn_client/checkout.c
> (svn_client__checkout_internal): We want to assert that 'url' is
> non-NULL, not 'URL', which is not initialized at that point.
(I haven't reviewed r12365 itself yet, so some of these comments are
probably really about that commit.)
May I suggest that having an incoming parameter named 'url' with an
internal variable named 'URL' is asking for trouble anyway? :-)
Also, looking at the code in svn_client__checkout_internal:
After this line
/* Canonicalize the URL. */
URL = svn_path_canonicalize (url, pool);
...there comes immediately an open-curly-brace (misindented, btw),
whose first bit of non-declaratory code is this:
/* Get the RA connection. */
SVN_ERR (svn_client__ra_lib_from_path (&ra_lib, &session, &revnum,
&URL, url, peg_revision,
revision, ctx, pool));
So, URL is being passed as the "return parameter" there, right after
we assigned URL a value (which will now be lost?). There's no
conditional around the call to svn_client__ra_lib_from_path(), and
nothing in that function's doc string indicates that the value of the
incoming URL is used in any way, nor that its assignment into URL is
conditional.
As I said, this properly belongs in a review of r12365, which will be
forthcoming. I just happened to notice it now, so am posting.
Best,
-Karl
> Modified: trunk/subversion/libsvn_client/checkout.c
> Url: http://svn.collab.net/viewcvs/svn/trunk/subversion/libsvn_client/checkout.c?view=diff&rev=12422&p1=trunk/subversion/libsvn_client/checkout.c&r1=12421&p2=trunk/subversion/libsvn_client/checkout.c&r2=12422
> ==============================================================================
> --- trunk/subversion/libsvn_client/checkout.c (original)
> +++ trunk/subversion/libsvn_client/checkout.c Sun Dec 19 19:23:56 2004
> @@ -60,7 +60,7 @@
>
> /* Sanity check. Without these, the checkout is meaningless. */
> assert (path != NULL);
> - assert (URL != NULL);
> + assert (url != NULL);
>
> /* Fulfill the docstring promise of svn_client_checkout: */
> if ((revision->kind != svn_opt_revision_number)
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: svn-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Dec 20 18:35:33 2004