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

Re: [PATCH] Please review : repositories recursive finding with SVNParentPath

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Thu, 9 Dec 2010 09:38:16 +0200

David Burley wrote on Wed, Dec 08, 2010 at 19:37:51 -0500:
> Patches attached again but can also be found here:
>
> http://subversion.tigris.org/issues/show_bug.cgi?id=3588
>
> On Wed, Dec 8, 2010 at 5:24 PM, Daniel Shahaf <d.s_at_daniel.shahaf.name>wrote:
>
> > David Burley wrote on Wed, Dec 08, 2010 at 17:03:06 -0500:
> > > So I have forward-ported Stephane's patch to add recursion to
> > SVNParentPath
> > > for 1.6.15 (see attached patch).
> >
> > The patch didn't get to the list. Please re-send it with *.txt extension.
> >
> > Thanks.
> >
> > Daniel
> >

The 1.7.x patch doesn't apply to HEAD of trunk.

> Index: subversion/mod_dav_svn/repos.c
> ===================================================================
> --- subversion/mod_dav_svn/repos.c (revision 1043620)
> +++ subversion/mod_dav_svn/repos.c (working copy)
> @@ -1235,34 +1235,63 @@
> {
> /* SVNParentPath was used instead: assume the first component of
> 'relative' is the name of a repository. */
> - const char *magic_component, *magic_end;
> + extern svn_boolean_t check_repos_path(const char *path, apr_pool_t *pool);
> + const char *magic_component = NULL, *magic_end;
> + char *repos_path;
>
> - /* A repository name is required here.
> - Remember that 'relative' always starts with a "/". */
> - if (relative[1] == '\0')
> - {
> - /* ### are SVN_ERR_APMOD codes within the right numeric space? */
> - return dav_svn__new_error(r->pool, HTTP_FORBIDDEN,
> - SVN_ERR_APMOD_MALFORMED_URI,
> - "The URI does not contain the name "
> - "of a repository.");
> - }
> + do
> + {
> + /* A repository name is required here.
> + Remember that 'relative' always starts with a "/". */
> + if (relative[1] == '\0')
> + {
> + /* ### are SVN_ERR_APMOD codes within the right numeric space? */
> + return dav_new_error(r->pool, HTTP_FORBIDDEN,
> + SVN_ERR_APMOD_MALFORMED_URI,
> + "The URI does not contain the name "
> + "of a repository.");
> + }
> +
> + magic_end = ap_strchr_c(relative + 1, '/');
> + if (!magic_end)
> + {
> + /* ### Request was for parent directory with no trailing
> + slash; we probably ought to just redirect to same with
> + trailing slash appended. */
> + if (!magic_component)
> + {
> + magic_component = relative + 1;
> + }
> + else
> + {
> + magic_component = apr_pstrcat(r->pool, magic_component,
> + relative, NULL);
> + }
> + relative = "/";
> + }
> + else
> + {
> + if (!magic_component)
> + {
> + magic_component = apr_pstrndup(r->pool, relative + 1,
> + magic_end - relative - 1);
> + }
> + else
> + {
> + char *tmp_magic_component;
>
> - magic_end = ap_strchr_c(relative + 1, '/');
> - if (!magic_end)
> - {
> - /* ### Request was for parent directory with no trailing
> - slash; we probably ought to just redirect to same with
> - trailing slash appended. */
> - magic_component = relative + 1;
> - relative = "/";
> - }
> - else
> - {
> - magic_component = apr_pstrndup(r->pool, relative + 1,
> - magic_end - relative - 1);
> - relative = magic_end;
> - }
> + tmp_magic_component = apr_pstrndup(r->pool, relative,
> + magic_end - relative);
> + magic_component = apr_pstrcat(r->pool, magic_component,
> + tmp_magic_component, NULL);
> + }
> + relative = magic_end;
> + }
> +
> + repos_path = svn_path_join(fs_parent_path, magic_component,
> + r->pool);
> + }
> + while (check_repos_path(repos_path, r->pool) != TRUE);
>
> /* return answer */
> *repos_basename = magic_component;
> @@ -1881,7 +1910,7 @@
> dav_resource_combined *comb;
> dav_svn_repos *repos;
> const char *cleaned_uri;
> - const char *repo_basename;
> + const char *repo_basename = NULL;
> const char *relative;
> const char *repos_path;
> const char *repos_key;
> @@ -1897,9 +1926,15 @@
> xslt_uri = dav_svn__get_xslt_uri(r);
> fs_parent_path = dav_svn__get_fs_parent_path(r);
>
> + /* This does all the work of interpreting/splitting the request uri. */
> + err = dav_svn_split_uri(r, r->uri, root_path,
> + &cleaned_uri, &had_slash,
> + &repo_basename, &relative, &repos_path);
> +
> +
> /* Special case: detect and build the SVNParentPath as a unique type
> of private resource, iff the SVNListParentPath directive is 'on'. */
> - if (fs_parent_path && dav_svn__get_list_parentpath_flag(r))
> + if (fs_parent_path && dav_svn__get_list_parentpath_flag(r) && !repo_basename)
> {
> char *uri = apr_pstrdup(r->pool, r->uri);
> char *parentpath = apr_pstrdup(r->pool, root_path);
> @@ -1912,13 +1947,10 @@
> if (parentpath[parentpath_len-1] == '/')
> parentpath[parentpath_len-1] = '\0';
>
> - if (strcmp(parentpath, uri) == 0)
> - {
> - err = get_parentpath_resource(r, resource);
> - if (err)
> - return err;
> - return NULL;
> - }
> + err = get_parentpath_resource(r, resource);
> + if (err)
> + return err;
> + return NULL;
> }
>
> /* This does all the work of interpreting/splitting the request uri. */
> @@ -3161,8 +3193,13 @@
> {
> apr_hash_index_t *hi;
> apr_hash_t *dirents;
> + int root_path_len = strlen(resource->info->repos->root_path);
> const char *fs_parent_path =
> - dav_svn__get_fs_parent_path(resource->info->r);
> + apr_pstrcat(resource->info->r->pool,
> + dav_svn__get_fs_parent_path(resource->info->r),
> + resource->info->r->uri
> + + ((root_path_len > 1) ? root_path_len : 0),
> + NULL);
>
> serr = svn_io_get_dirents3(&dirents, fs_parent_path, TRUE,
> resource->pool, resource->pool);
> Index: subversion/libsvn_repos/repos.c
> ===================================================================
> --- subversion/libsvn_repos/repos.c (revision 1043620)
> +++ subversion/libsvn_repos/repos.c (working copy)
> @@ -1428,7 +1428,7 @@
> * on errors (which would be permission errors, probably) so that
> * we the user will see them after we try to open the repository
> * for real. */
> -static svn_boolean_t
> +svn_boolean_t
> check_repos_path(const char *path,
> apr_pool_t *pool)

As earlier reviewers told you, we do not define functions in one library
and use them in another unless they have a svn_ svn_foo__ name prefix.

> {
>
Received on 2010-12-09 08:40:40 CET

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.