Re: [PATCH] svn_path_canonicalize

I haven't really looked at this patch, but I have a question already. There
are several places where svn_path_canonicalize are called from within other
parts of the path library. I was thinking we could make url_canonicalize
private to path.c and make svn_path_canonicalize look like:

if (style == svn_path_url_style)
  url_canonicalize(path);
else
  {
    /* Do our non-url canonicalization. */
  }

This also means nothing outside of path.c has to change.

Other's thoughts?

On Tue, Sep 18, 2001 at 11:48:44PM -0400, Garrett Rooney wrote:
> this patch makes svn_path_canonicalize remove redundant dirseps as
> well as removing extra dirseps at the end of a path. In addition, it
> implememnts svn_url_canonicalize, which does the same for url's, since
> it seems there are several places in the code where we were passing
> url's to svn_path_canonicalize. note, i have grepped the source tree
> for instances of svn_path_canonicalize and changed those i found that
> should use the url version. it is possible that i've missed some
> though, specifically in the server side stuff, as i don't currently
> have a server set up to test with. someone familiar with the server
> side code should probably give this a close look.
>
> * path.c (condense_path): add new helper function.
>
> * path.c (svn_path_canonicalize): use condense_path to remove
> redundant portions of the path.
>
> * path.c (svn_url_canonicalize): new function to handle cases where
> urls were being passed to svn_path_canonicalize.
>
> * path.c (add_component_internal): remove trailing dirseps ourselves
> instead of using svn_path_canonicalize because we cannot be sure if
> we're being called with a path or a url.
>
> * path.c (svn_path_remove_component): ditto.
>
> * path.c (svn_path_split): ditto.
>
> * svn_path.h: add prototype for svn_url_canonicalize
>
> * checkout-cmd.c (svn_cl__checkout): use svn_url_canonicalize instead
> of svn_path_canonicalize since this is a url.
>
> * checkout.c (svn_client_checkout): ditto.
>
> * entries.c (svn_wc__compose_paths): ditto.
>
> --
> 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.

> Index: subversion/clients/cmdline/checkout-cmd.c
> ===================================================================
> --- subversion/clients/cmdline/SVN/text-base/checkout-cmd.c Tue Sep 18 17:39:00 2001
> +++ subversion/clients/cmdline/checkout-cmd.c Tue Sep 18 22:45:44 2001
> @@ -99,7 +99,7 @@
> = ((svn_stringbuf_t **) (opt_state->args->elts))[0];
>
> /* Canonicalize the URL. */
> - svn_path_canonicalize (repos_url, svn_path_repos_style);
> + svn_url_canonicalize (repos_url, svn_path_repos_style);
>
> /* Ensure that we have a default dir to checkout into. */
> if (! opt_state->target)
> Index: subversion/include/svn_path.h
> ===================================================================
> --- subversion/include/SVN/text-base/svn_path.h Mon Sep 10 23:10:58 2001
> +++ subversion/include/svn_path.h Tue Sep 18 23:28:53 2001
> @@ -111,12 +111,16 @@
> int svn_path_is_empty (const svn_stringbuf_t *path, enum svn_path_style style);
>
>
> -/* Remove trailing separators that don't affect the meaning of the path.
> - (At some future point, this may make other semantically inoperative
> - transformations.) */
> +/* Remove trailing separators and redundant entries that don't affect the
> + meaning of the path. */
> void svn_path_canonicalize (svn_stringbuf_t *path,
> enum svn_path_style style);
>
> +
> +/* Remove trailing separators and redundant entries that don't affect the
> + meaning of the url. */
> +void svn_url_canonicalize (svn_stringbuf_t *path,
> + enum svn_path_style style);
>
> /* Return an integer greater than, equal to, or less than 0, according
> as PATH1 is greater than, equal to, or less than PATH2. */
> Index: subversion/libsvn_client/checkout.c
> ===================================================================
> --- subversion/libsvn_client/SVN/text-base/checkout.c Tue Sep 18 17:38:58 2001
> +++ subversion/libsvn_client/checkout.c Tue Sep 18 22:42:31 2001
> @@ -58,7 +58,7 @@
> assert (URL != NULL);
>
> /* Canonicalize the URL. */
> - svn_path_canonicalize (URL, svn_path_repos_style);
> + svn_url_canonicalize (URL, svn_path_repos_style);
>
> /* Fetch the checkout editor. If REVISION is invalid, that's okay;
> either the RA or XML driver will call editor->set_target_revision
> Index: subversion/libsvn_subr/path.c
> ===================================================================
> --- subversion/libsvn_subr/SVN/text-base/path.c Tue Sep 18 17:38:56 2001
> +++ subversion/libsvn_subr/path.c Tue Sep 18 23:18:54 2001
> @@ -69,16 +69,31 @@
>
>
> 
> +static void
> +condense_path(char **path, enum svn_path_style style)
> +{
> + char dirsep = get_separator_from_style (style);
> +
> + char sep_sep[3] = { dirsep, dirsep, '\0' };
> + char sep_dot_sep[4] = { dirsep, '.', dirsep, '\0' };
> +
> + char *tmp = NULL;
> +
> + while ((tmp = strstr (*path, sep_sep)) != NULL)
> + strcpy(tmp, tmp + 1);
> +
> + while ((tmp = strstr (*path, sep_dot_sep)) != NULL)
> + strcpy(tmp, tmp + 2);
> +}
> +
> void
> svn_path_canonicalize (svn_stringbuf_t *path, enum svn_path_style style)
> {
> 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. */
> + condense_path(&path->data, style);
>
> - /* kff todo: maybe should be implemented with a new routine in
> - libsvn_string. */
> + path->len = strlen (path->data);
>
> while ((path->len > 0)
> && (path->data[(path->len - 1)] == dirsep))
> @@ -88,6 +103,26 @@
> }
> }
>
> +void
> +svn_url_canonicalize (svn_stringbuf_t *path, enum svn_path_style style)
> +{
> + char dirsep = get_separator_from_style (style);
> + char * tmp;
> +
> + tmp = strstr(path->data, "//");
> + tmp += 2;
> +
> + condense_path(&tmp, style);
> +
> + path->len = strlen(path->data);
> +
> + while ((path->len > 0)
> + && (path->data[(path->len - 1)] == dirsep))
> + {
> + path->data[(path->len - 1)] = '\0';
> + path->len--;
> + }
> +}
>
> static void
> add_component_internal (svn_stringbuf_t *path,
> @@ -101,7 +136,13 @@
> svn_stringbuf_appendbytes (path, &dirsep, sizeof (dirsep));
>
> svn_stringbuf_appendbytes (path, component, len);
> - svn_path_canonicalize (path, style);
> +
> + while ((path->len > 0)
> + && (path->data[(path->len - 1)] == dirsep))
> + {
> + path->data[(path->len - 1)] = '\0';
> + path->len--;
> + }
> }
>
>
> @@ -128,8 +169,13 @@
> {
> char dirsep = get_separator_from_style (style);
>
> - svn_path_canonicalize (path, style);
> -
> + while ((path->len > 0)
> + && (path->data[(path->len - 1)] == dirsep))
> + {
> + path->data[(path->len - 1)] = '\0';
> + path->len--;
> + }
> +
> if (! svn_stringbuf_chop_back_to_char (path, dirsep))
> svn_stringbuf_setempty (path);
> else
> @@ -169,11 +215,17 @@
> apr_pool_t *pool)
> {
> svn_stringbuf_t *n_dirpath, *n_basename;
> + char dirsep = get_separator_from_style (style);
>
> assert (dirpath != basename);
>
> n_dirpath = svn_stringbuf_dup (path, pool);
> - svn_path_canonicalize (n_dirpath, style);
> + while ((n_dirpath->len > 0)
> + && (n_dirpath->data[(n_dirpath->len - 1)] == dirsep))
> + {
> + n_dirpath->data[(n_dirpath->len - 1)] = '\0';
> + n_dirpath->len--;
> + }
>
> if (basename)
> n_basename = svn_path_last_component (n_dirpath, style, pool);
> @@ -280,7 +332,13 @@
> else
> common_path = svn_stringbuf_ncreate (path1->data, last_dirsep, pool);
>
> - svn_path_canonicalize (common_path, style);
> + while ((common_path->len > 0)
> + && (common_path->data[(common_path->len - 1)] == dirsep))
> + {
> + common_path->data[(common_path->len - 1)] = '\0';
> + common_path->len--;
> + }
> +
>
> return common_path;
> }
> Index: subversion/libsvn_wc/entries.c
> ===================================================================
> --- subversion/libsvn_wc/SVN/text-base/entries.c Mon Sep 10 23:10:25 2001
> +++ subversion/libsvn_wc/entries.c Tue Sep 18 23:26:51 2001
> @@ -1616,7 +1616,7 @@
> path = val;
>
> apr_hash_set (paths, key, keylen, NULL);
> - svn_path_canonicalize (path, svn_path_local_style);
> + svn_url_canonicalize (path, svn_path_local_style);
> apr_hash_set (paths, path->data, path->len, path);
> }
>
>

> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org

-- 
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Kevin Pilch-Bisson                    http://www.pilch-bisson.net
     "Historically speaking, the presences of wheels in Unix
     has never precluded their reinvention." - Larry Wall
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~