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

Re: [PATCH] issue 1954 - v3

From: Peter N. Lundblad <peter_at_famlundblad.se>
Date: 2004-12-14 10:05:39 CET

On Tue, 14 Dec 2004, VK Sameer wrote:

In addition to Karl's review:

Index: subversion/include/svn_path.h
===================================================================
--- subversion/include/svn_path.h (revision 12257)
+++ subversion/include/svn_path.h (working copy)
@@ -356,6 +356,26 @@
                                const char *path2,
                                apr_pool_t *pool);

+/**
+ * @since New in 1.2.
+ *
+ * Check whether @a path is a valid Subversion path.
+ * @a pool is used for temporary allocation.
+ *
+ * A valid Subversion pathname is a UTF-8 string without control
+ * characters. "Valid" means Subversion can store the pathname in
+ * a repository. There may be other OS-specific limitations on what
+ * paths can be represented in a working copy.
+ *
+ * returns SVN_NO_ERROR if valid
+ * returns SVN_ERR_FS_PATH_SYNTAX if invalid

Not sure how this will be formatted in the doxygen output. I assume it
won't take the line breaks into account, but I haven't checked.

Index: subversion/libsvn_subr/path.c
===================================================================
--- subversion/libsvn_subr/path.c (revision 12257)
+++ 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"

 /* The canonical empty path. Can this be changed? Well, change the empty
@@ -1248,3 +1249,22 @@
   else
     return svn_utf_cstring_to_utf8 (path_utf8, path_apr, pool);
 }
+
+svn_error_t*

Space after *.

+svn_path_check_valid (const char *path, apr_pool_t *pool)
+{
+ const char *c;
+
+ for (c = path; *c; c++)
+ {
+ if (svn_ctype_iscntrl(*c))
+ {
+ return svn_error_createf (SVN_ERR_FS_PATH_SYNTAX, NULL,
+ "Invalid control char. '%x' in path '%s'",
+ *c,
+ svn_path_local_style (path, pool));

Since 1.1, svn is internationalized, so you need to put strings that
should be translated into the users' native language inside _() macro
calls. + }

Index: subversion/libsvn_ra_local/ra_plugin.c
===================================================================
--- subversion/libsvn_ra_local/ra_plugin.c (revision 12257)
+++ subversion/libsvn_ra_local/ra_plugin.c (working copy)
@@ -688,6 +688,9 @@
   svn_fs_root_t *root;
   const char *abs_path = sbaton->fs_path;

+ if (path)
+ SVN_ERR (svn_path_check_valid (path, pool));
+

svn_ra_plugin_t->check_path, which this implements, takes a path, checks
if it exists (in a particular revision) and what kind it is. So you don't
need the above.

I see that you are adding the checks to both libsvn_wc and libsvn_client.
Isn't it enough to add them to libsvn_wc (except in the import code)? For
example, wc_to_wc_copy it seems redundant. Or maybe you are plugging some
hole somewhere else?

Thanks,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Dec 14 10:06:08 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.