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

Re: svn commit: r32636 - in trunk/subversion: libsvn_repos tests/cmdline

From: Hyrum K. Wright <hyrum_wright_at_mail.utexas.edu>
Date: Fri, 22 Aug 2008 16:50:31 -0700

Karl Fogel wrote:
> hwright_at_tigris.org writes:
>> --- trunk/subversion/libsvn_repos/repos.c
>> +++ trunk/subversion/libsvn_repos/repos.c
>> @@ -1192,6 +1192,7 @@ svn_repos_create(svn_repos_t **repos_p,
>> {
>> svn_repos_t *repos;
>> svn_error_t *err;
>> + const char *root_path;
>>
>> /* Allocate a repository object, filling in the format we will create. */
>> repos = create_svn_repos_t(path, pool);
>> @@ -1210,6 +1211,13 @@ svn_repos_create(svn_repos_t **repos_p,
>> if (! repos->fs_type)
>> repos->fs_type = DEFAULT_FS_TYPE;
>>
>> + /* Don't create a repository inside another repository. */
>> + root_path = svn_repos_find_root_path(path, pool);
>> + if (root_path != NULL)
>> + return svn_error_createf(SVN_ERR_REPOS_BAD_ARGS, NULL, _("'%s' is a "
>> + "subdirectory of an existing repository rooted "
>> + "at '%s'"), path, root_path);
>> +
>> /* Create the various files and subdirectories for the repository. */
>> SVN_ERR_W(create_repos_structure(repos, path, fs_config, pool),
>> _("Repository creation failed"));
>
> The change looks good to me (with r32639), and I've voted for the group
> in 1.5.x/STATUS. But, do you know why svn_repos_find_root_path()
> doesn't return 'svn_error_t *' directly and return the result path by
> reference, like one would normally expect?

I'm not sure why svn_repos_find_root_path() doesn't return an svn_error_t *. My
suspicion would be that it is a hold over from the pre-1.0 days when we were
less strict about returning svn_error_t *. I didn't really need the error, and
I wasn't thrilled about rev'ing the API for this change, so I didn't pursue it
further.

> I was initially going to suggest getting rid of the 'root_path' variable
> and just using the result of the function call directly in the
> conditional. But then I thought "Wait, why the heck is that prototype
> returning the path directly anyway?"... which led me to ask this
> question here.

I initially wrote the patch to not use the root_path variable, but decided that
it would be useful to have in the error message. We can remove it if it isn't
required in the message.

-Hyrum

Received on 2008-08-23 01:50:55 CEST

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.