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

Re: [PATCH] fix for issue #2556: support working copies on the root of a (virtual) drive

From: Erik Huelsmann <ehuels_at_gmail.com>
Date: 2006-08-21 21:59:29 CEST

On 8/21/06, Lieven Govaerts <lgo@mobsol.be> wrote:
> Attached is a new version of these patches. It's based on the remarks all of
> you made the previous days.
>
> Changes compared to the previous version include:
> - use apr's apr_filepath_root for a portable way to determine if a path is a
> root path.
> - an expanded python test so more problem cases could be solved
> - unit tests (test_path) to validate the changes in the svn_path functions.
>
> The 'working copy is switched' is unchanged compared to previous version.
>
> As I said before, there are probably other scenario's of using wc's on root
> folders we can find that won't work with current patch. I'll try to find
> these scenario's, add them to the python tests and fix the issues in the
> coming days/weeks.

Ok. I didn't investigate more of it myself, so I rely on you to make
sure that we call svn_path_is_root from other functions which don't
have a pool (excluding is_canonical, since Philip said we can safely
remove that function from consideration).

However, calling svn_pool_create(NULL) isn't thread safe AFAIK,
because it allocates from the main pool (is this right, Branko,
anybody?). Or is that the only bit for which the main pool *is*
protected?

Anyway: two other comments inline below:

@@ -418,6 +424,35 @@
 }

+svn_boolean_t
+svn_path_is_root(const char *path, apr_size_t len)
+{
+ char *root_path = NULL;
+ apr_status_t status;
+ apr_pool_t *strpool = svn_pool_create(NULL);
+ char *rel_path = apr_pstrmemdup(strpool, path, len);
+
+ /* svn_path_cstring_from_utf8 will create a copy of path.
+
+ It should be safe to convert this error to a false return value. An error
+ in this case would indicate that the path isn't encoded in UTF-8, which
+ will cause problems elsewhere, anyway. */
+ if (svn_path_cstring_from_utf8(&rel_path, rel_path, strpool) != APR_SUCCESS)

Here you may leek a pool.

+ return FALSE;
+
+ status = apr_filepath_root(&root_path, &rel_path, 0, strpool);
+
+ apr_pool_destroy(strpool);
+
+ if ((status == APR_SUCCESS ||
+ status == APR_EINCOMPLETE) &&
+ rel_path[0] == '\0')

Here you access memory which you may have destroyed in
apr_pool_destroy: rel_path may have been allocated in the strpool,
right?

+ return TRUE;
+
+ return FALSE;
+}
+
+
 int
 svn_path_compare_paths(const char *path1,
                        const char *path2)

Thanks for your perseverance.

bye,

Erik.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Aug 21 22:19:23 2006

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.