On Sat, 2005-09-03 at 18:29 +0100, Julian Foad wrote:
> Alexander, ping! I've addressed most of Peter's concerns. Can you comment on
> the rest?
>
>
> Peter N. Lundblad wrote:
> > On Thu, 11 Aug 2005 sussman@tigris.org wrote:
>
> > r15678 | sussman | 2005-08-11 15:02:04 +0100 (Thu, 11 Aug 2005) | 25 lines
> >
> > Fix issue #2291: 'svn ls -v' should show the existence of repository locks.
> > Slightly tweaked by sussman.
> >
> > Patch by: Alexander Thomas <alexander@collab.net>
> > Review by: sussman
> > lundblad
>
> [...]
>
> >>+ SVN_ERR (svn_ra_get_locks (ra_session, locks, "", pool));
> >>+
> >>+ new_locks = apr_hash_make (pool);
> >>+ for (hi = apr_hash_first (pool, *locks); hi; hi = apr_hash_next (hi))
> >>+ {
> >>+ const void *key;
> >>+ void *val;
> >>+ const char *newkey;
> >>+
> >>+ svn_pool_clear (subpool);
> >>+
> >>+ apr_hash_this (hi, &key, NULL, &val);
> >>+ newkey = svn_path_is_child (svn_path_canonicalize (rel_path, subpool),
> >>+ svn_path_canonicalize (key, subpool),
> >>+ pool);
> >
> > Why canonicalize here? If rel_path needs that, it could be done outside of
> > the loop, but why are we creating non-canonical paths in the first place?
> > And regarding key, doesn't the API guarantee that that is canonicalized?
>
> The trailing slash and the canonicalisation appear to be unnecessary.
> Alexander, can you comment?
>
Yes both are unnecessary. I misread docs on svn_path_xxxxxx().
> I attach a patch which addresses everything here (and a bit more). Is it correct?
>
I test it to my satisfaction, Prefect. +1
Thanks
-Alexander Thomas (AT)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Sep 5 12:15:37 2005