[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] First cut at 1954 solution

From: C. Michael Pilato <cmpilato_at_collab.net>
Date: 2004-12-06 15:18:57 CET

VK Sameer <sameer@collab.net> writes:

> Index: subversion/include/svn_path.h
> ===================================================================
> --- subversion/include/svn_path.h (revision 12179)
> +++ subversion/include/svn_path.h (working copy)
> @@ -338,6 +338,22 @@
> svn_boolean_t svn_path_is_backpath_present (const char *path);
>
>
> +/**
> + * @since New in 1.2.
> + *
> + * Test to see if a path is allowed in a Subversion repository.

Actually, the question is whether or not a path is allowed in a
Subversion working copy. But the generic statement is more like,
"Check @a path to see if it is supported as the path of a versioned
resource by Subvesion."

> + * First check whether all characters are valid in UTF8
> + * Next check whether all characters are non-control characters
> + * Currently, all other paths are allowed.

Hrm... this is somewhere between "leaking the details of the
implmentation" and "actually useful information". I think it's the
"first...next..." that's bugging me.

> + *
> + * This is OS-independent - it is used to decide what paths may be
> + * added to a repository. Adding occurs either using a file on disk
> + * (svn import/add) or one not on disk (svn mv/copy URL1 URL2).
> + */

Lose this. Subversion APIs are OS-independent unless otherwise
stated, and it's entirely unimportant when adding occurs.

> +svn_error_t *svn_path_is_valid_in_svn (const char *path,
> + apr_size_t len,
> + apr_pool_t *pool);
> +

Judging by the signature, I'm guessing that the function returns
SVN_NO_ERROR if PATH is all good, and some other error otherwise. You
might want to state as much, so callers of the API can tell the
difference between an "unsupported path" error and some other error.
Also, we don't generally pass lengths with our paths.

> Index: subversion/libsvn_subr/path.c
> ===================================================================
> --- subversion/libsvn_subr/path.c (revision 12179)
> +++ subversion/libsvn_subr/path.c (working copy)
> @@ -29,6 +29,7 @@
> #include "svn_private_config.h" /* for SVN_PATH_LOCAL_SEPARATOR */
> #include "svn_utf.h"
> #include "svn_io.h" /* for svn_io_stat() */
> +#include "svn_ctype.h" /* for svn_ctype_() */
>
>
> /* The canonical empty path. Can this be changed? Well, change the empty
> @@ -1248,3 +1249,25 @@
> else
> return svn_utf_cstring_to_utf8 (path_utf8, path_apr, pool);
> }
> +
> +svn_error_t*
> +svn_path_is_valid_in_svn (const char *path,
> + apr_size_t len,
> + apr_pool_t *pool)
> +{
> + int i;
> + for (i = 0; i < len; i++)
> + {

Yep, no need for that LEN parameter.

   const char *c = path;
   while (*c)
     {
       if (!svn_ctype_isutf8(*c))
       ...
       c++;
     }

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Dec 6 15:49:09 2004

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.