Re: [PATCH] svn_path_canonicalize updated

On Wed, Sep 19, 2001 at 11:13:47PM -0700, Greg Stein wrote:
> On Wed, Sep 19, 2001 at 10:39:45PM -0400, Garrett Rooney wrote:
> >...
> > --- subversion/clients/cmdline/SVN/text-base/util.c Mon Sep 10 23:07:52 2001
> > +++ subversion/clients/cmdline/util.c Wed Sep 19 21:05:13 2001
> > @@ -158,7 +158,6 @@
> > svn_stringbuf_t *target = svn_stringbuf_create (os->argv[os->ind], pool);
> > svn_stringbuf_t *basename;
> >
> > - svn_path_canonicalize (target, svn_path_local_style);
> > basename = svn_path_last_component (target, svn_path_local_style, pool);
>
> There are probably some repercussions from doing this. Note that the next
> call is to svn_path_last_component(). That function may fail if passed a
> non-canonical path.
>
> Also note that it uses svn_path_local_style, too. IOW, it will be just as
> broken as the call to the canonicalize. Somebody needs to get in there and
> do some real review and fixing.

ack, you're right... this is ugly... i'll have to take a closer
look...

> >...
> > --- subversion/libsvn_subr/SVN/text-base/path.c Tue Sep 18 17:38:56 2001
> > +++ subversion/libsvn_subr/path.c Wed Sep 19 20:55:17 2001
> > @@ -74,11 +74,28 @@
> > {
> > char dirsep = get_separator_from_style (style);
> >
> > - /* At some point this could eliminiate redundant components.
> > - For now, it just makes sure there is no trailing slash. */
> > + char sep_sep[3] = { dirsep, dirsep, '\0' };
> > + char sep_dot_sep[0] = { dirsep, '.', dirsep, '\0' };
> >
> > - /* kff todo: maybe should be implemented with a new routine in
> > - libsvn_string. */
> > + char *str = NULL, *tmp = NULL;
> > +
> > + if (style == svn_path_url_style || style == svn_path_repos_style) {
> > + str = strstr (path->data, "//");
>
> A relative URL with "//" present will throw this off.

how should i tell the difference between an absolute url and a
relative url? i suppose i could do a direct compare against http://
https:// and file://, but that seems ugly...

> >...
> > + while ((tmp = strstr (str, sep_sep)) != NULL)
> > + strcpy (tmp, tmp + 1);
> > +
> > + while ((tmp = strstr (str, sep_dot_sep)) != NULL)
> > + strcpy (tmp, tmp + 2);
>
> strcpy() is not guaranteed to work properly when the two strings overlap.

good point, i'll change that to use memmove, which is guaranteed...

thanks,

-garrett

-- 
garrett rooney                     Unix was not designed to stop you from 
rooneg@electricjellyfish.net       doing stupid things, because that would  
http://electricjellyfish.net/      stop you from doing clever things.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org