Philip Martin <philip.martin_at_wandisco.com> writes:
> brane_at_apache.org writes:
>
>> Author: brane
>> Date: Thu Apr 25 13:48:46 2013
>> New Revision: 1475772
>>
>> URL: http://svn.apache.org/r1475772
>> Log:
>> Fix issue #4361 (user/group names in authz files are case-insensitive).
>>
>> * subversion/include/svn_config.h
>> (svn_config_create2): New, revised from svn_config_create, with additional
>> flag to treat option names as case-sensitive.
>> (svn_config_read3): New, revised from svn_config_read2, with same
>> additional flag as above.
>> (svn_config_parse): Add flag to treat option names as case-sensitive.
>> (svn_config_create): Deprecated.
>> (svn_config_read2): Deprecated; argument "pool" renamde to "result_pool".
>> (svn_config_read): Argument "pool" renamde to "result_pool".
>
> svn_config_read3 and svn_config_parse are both new in 1.8, should we add
> a scratch_pool before releasing these APIs? At present they both create
> an internal scratch pool as a subpool of result_pool.
This turned out to be a more invasive change than I had anticipated.
The scratch pool parameter spread to the four new-in-1.8 functions:
svn_config_read3, svn_config_parse, svn_repos_authz_read2 and
svn_repos_authz_parse.
What makes it tricky is that the old implementation creates and destroys
local scratch pools as subpools of the result pool in two places:
svn_config__parse_file and authz_retrieve_config. This controls file
handles and perhaps memory use as well. I'm not sure how these cases
should be handled with a new scratch pool API. Ideally these subpools
would be removed and the caller would be responsible for clearing the
scratch pool. It's simple enough to make the legacy API create a subpool
but direct callers of the new scratch pool API would have to make sure
that the scratch pool had an appropriate lifetime. (Imagine
mod_authz_svn parsing a large authz file, if the scratch pool lifetime
is significantly longer than the old subpool lifetime then we may have a
memory use problem.)
My current patch both adds the scratch pool and keeps the local subpool
creation so it should not have any significant memory effects. Ideally
we would remove the local subpools as well but that requires auditing
the callers and understanding the resource usage. If we have made the
API change we can improve the implementation in later releases.
The reason I hesitate is that we are so close to a 1.8 release. Do
people want this patch in 1.8?
Add a scratch pool to the new-in-1.8 svn_config_read3, svn_config_parse,
svn_repos_authz_read2 and svn_repos_authz_parse.
* subversion/include/svn_config.h
(svn_config_read3, svn_config_parse): Add scratch pool.
* subversion/include/svn_repos.h
(svn_repos_authz_read2, svn_repos_authz_parse): Add scratch pool.
* subversion/libsvn_subr/deprecated.c
(svn_config_read2): Create/destroy scratch pool.
(svn_config_read): Call svn_config_read2.
* subversion/libsvn_repos/deprecated.c
(svn_repos_authz_read): Create/destroy scratch pool.
* subversion/libsvn_subr/config_impl.h
(svn_config__parse_file, svn_config__parse_registry): Rename pool
parameter to scratch pool.
(svn_config__parse_stream): Remove unused result pool.
* subversion/libsvn_subr/config.c
(svn_config_read3, svn_config_parse): Add scratch pool,
(read_all): Create a scratch pool.
(svn_config_merge): Pass config pool as scratch pool.
* subversion/libsvn_subr/config_file.c
(svn_config__parse_file): Rename pool parameter, rename local scratch pool.
(svn_config__parse_stream): Remove unused result pool.
* subversion/libsvn_subr/config_win.c
(svn_config__parse_registry): Rename pool parameter.
* subversion/libsvn_repos/repos.h
(svn_repos__authz_read): Add scratch pool.
* subversion/libsvn_repos/authz.c
(authz_retrieve_config_repo): Pass scratch pool.
(authz_retrieve_config): Add scratch pool, rename local scratch pool.
(svn_repos__authz_read, svn_repos_authz_read2): Add scratch pool.
* subversion/libsvn_repos/hooks.c
(svn_repos__parse_hooks_env): Pass scratch pool.
* subversion/mod_authz_svn/mod_authz_svn.c
(get_access_conf): Pass scratch pool.
* subversion/svnserve/serve.c
(load_pwdb_config, load_authz_config): Pass pool as scratch pool.
* subversion/svnserve/svnserve.c
(main): Pass pool as scratch pool.
* tools/server-side/mod_dontdothat/mod_dontdothat.c
(dontdothat_insert_filters): Pass request pool as scratch pool.
* tools/server-side/svnauthz.c
(get_authz_from_txn, get_authz): Pass pool as scratch pool.
* subversion/libsvn_fs_fs/fs_fs.c
(read_config): Pass pool as scratch pool.
* subversion/tests/libsvn_subr/config-test.c
(test_text_retrieval, test_boolean_retrieval,
test_has_section_case_insensitive, test_has_section_case_sensitive,
test_has_option_case_sensitive, test_stream_interface): Adjust call.
* subversion/tests/libsvn_subr/cache-test.c
(test_memcache_basic, test_memcache_long_key): Adjust call.
* subversion/tests/libsvn_repos/repos-test.c
(authz_get_handle, in_repo_authz, in_repo_groups_authz,
authz_groups_get_handle): Adjust call.
Index: subversion/include/svn_config.h
===================================================================
--- subversion/include/svn_config.h (revision 1477539)
+++ subversion/include/svn_config.h (working copy)
@@ -260,7 +260,8 @@ svn_config_read3(svn_config_t **cfgp,
svn_boolean_t must_exist,
svn_boolean_t section_names_case_sensitive,
svn_boolean_t option_names_case_sensitive,
- apr_pool_t *result_pool);
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool);
/** Similar to svn_config_read3, but always passes @c FALSE to
* @a option_names_case_sensitive.
@@ -305,7 +306,8 @@ svn_config_parse(svn_config_t **cfgp,
svn_stream_t *stream,
svn_boolean_t section_names_case_sensitive,
svn_boolean_t option_names_case_sensitive,
- apr_pool_t *result_pool);
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool);
/** Like svn_config_read(), but merges the configuration data from @a file
* (a file or registry path) into @a *cfg, which was previously returned
Index: subversion/include/svn_repos.h
===================================================================
--- subversion/include/svn_repos.h (revision 1477539)
+++ subversion/include/svn_repos.h (working copy)
@@ -3230,7 +3230,8 @@ svn_repos_authz_read2(svn_authz_t **authz_p,
const char *path,
const char *groups_path,
svn_boolean_t must_exist,
- apr_pool_t *pool);
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool);
/**
@@ -3259,7 +3260,8 @@ svn_error_t *
svn_repos_authz_parse(svn_authz_t **authz_p,
svn_stream_t *stream,
svn_stream_t *groups_stream,
- apr_pool_t *pool);
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool);
/**
* Check whether @a user can access @a path in the repository @a
Index: subversion/libsvn_fs_fs/fs_fs.c
===================================================================
--- subversion/libsvn_fs_fs/fs_fs.c (revision 1477539)
+++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
@@ -1159,7 +1159,7 @@ read_config(fs_fs_data_t *ffd,
{
SVN_ERR(svn_config_read3(&ffd->config,
svn_dirent_join(fs_path, PATH_CONFIG, pool),
- FALSE, FALSE, FALSE, pool));
+ FALSE, FALSE, FALSE, pool, pool));
/* Initialize ffd->rep_sharing_allowed. */
if (ffd->format >= SVN_FS_FS__MIN_REP_SHARING_FORMAT)
Index: subversion/libsvn_repos/authz.c
===================================================================
--- subversion/libsvn_repos/authz.c (revision 1477539)
+++ subversion/libsvn_repos/authz.c (working copy)
@@ -842,7 +842,8 @@ authz_retrieve_config_repo(svn_config_t **cfg_p, c
}
SVN_ERR(svn_fs_file_contents(&contents, root, fs_path, scratch_pool));
- err = svn_config_parse(cfg_p, contents, TRUE, TRUE, result_pool);
+ err = svn_config_parse(cfg_p, contents, TRUE, TRUE,
+ result_pool, scratch_pool);
/* Add the URL to the error stack since the parser doesn't have it. */
if (err != SVN_NO_ERROR)
@@ -869,22 +870,22 @@ authz_retrieve_config_repo(svn_config_t **cfg_p, c
* don't have a repos relative URL in PATH. */
static svn_error_t *
authz_retrieve_config(svn_config_t **cfg_p, const char *path,
- svn_boolean_t must_exist, apr_pool_t *pool)
+ svn_boolean_t must_exist, apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool)
{
if (svn_path_is_url(path))
{
const char *dirent;
svn_error_t *err;
- apr_pool_t *scratch_pool = svn_pool_create(pool);
+ apr_pool_t *subpool = svn_pool_create(scratch_pool);
- err = svn_uri_get_dirent_from_file_url(&dirent, path, scratch_pool);
+ err = svn_uri_get_dirent_from_file_url(&dirent, path, subpool);
if (err == SVN_NO_ERROR)
- err = authz_retrieve_config_repo(cfg_p, dirent, must_exist, pool,
- scratch_pool);
-
+ err = authz_retrieve_config_repo(cfg_p, dirent, must_exist,
+ result_pool, subpool);
/* Close the repos and streams we opened. */
- svn_pool_destroy(scratch_pool);
+ svn_pool_destroy(subpool);
return err;
}
@@ -891,7 +892,8 @@ authz_retrieve_config(svn_config_t **cfg_p, const
else
{
/* Outside of repo file or Windows registry*/
- SVN_ERR(svn_config_read3(cfg_p, path, must_exist, TRUE, TRUE, pool));
+ SVN_ERR(svn_config_read3(cfg_p, path, must_exist, TRUE, TRUE,
+ result_pool, scratch_pool));
}
return SVN_NO_ERROR;
@@ -936,15 +938,18 @@ authz_copy_groups(svn_authz_t *authz, svn_config_t
svn_error_t *
svn_repos__authz_read(svn_authz_t **authz_p, const char *path,
const char *groups_path, svn_boolean_t must_exist,
- svn_boolean_t accept_urls, apr_pool_t *pool)
+ svn_boolean_t accept_urls, apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool)
{
- svn_authz_t *authz = apr_palloc(pool, sizeof(*authz));
+ svn_authz_t *authz = apr_palloc(result_pool, sizeof(*authz));
/* Load the authz file */
if (accept_urls)
- SVN_ERR(authz_retrieve_config(&authz->cfg, path, must_exist, pool));
+ SVN_ERR(authz_retrieve_config(&authz->cfg, path, must_exist,
+ result_pool, scratch_pool));
else
- SVN_ERR(svn_config_read3(&authz->cfg, path, must_exist, TRUE, TRUE, pool));
+ SVN_ERR(svn_config_read3(&authz->cfg, path, must_exist, TRUE, TRUE,
+ result_pool, scratch_pool));
if (groups_path)
{
@@ -954,13 +959,13 @@ svn_repos__authz_read(svn_authz_t **authz_p, const
/* Load the groups file */
if (accept_urls)
SVN_ERR(authz_retrieve_config(&groups_cfg, groups_path, must_exist,
- pool));
+ result_pool, scratch_pool));
else
SVN_ERR(svn_config_read3(&groups_cfg, groups_path, must_exist,
- TRUE, TRUE, pool));
+ TRUE, TRUE, result_pool, scratch_pool));
/* Copy the groups from groups_cfg into authz. */
- err = authz_copy_groups(authz, groups_cfg, pool);
+ err = authz_copy_groups(authz, groups_cfg, scratch_pool);
/* Add the paths to the error stack since the authz_copy_groups
routine knows nothing about them. */
@@ -971,7 +976,7 @@ svn_repos__authz_read(svn_authz_t **authz_p, const
}
/* Make sure there are no errors in the configuration. */
- SVN_ERR(authz_validate(authz, pool));
+ SVN_ERR(authz_validate(authz, scratch_pool));
*authz_p = authz;
return SVN_NO_ERROR;
@@ -984,21 +989,23 @@ svn_repos__authz_read(svn_authz_t **authz_p, const
svn_error_t *
svn_repos_authz_read2(svn_authz_t **authz_p, const char *path,
const char *groups_path, svn_boolean_t must_exist,
- apr_pool_t *pool)
+ apr_pool_t *result_pool, apr_pool_t *scratch_pool)
{
return svn_repos__authz_read(authz_p, path, groups_path, must_exist,
- TRUE, pool);
+ TRUE, result_pool, scratch_pool);
}
svn_error_t *
svn_repos_authz_parse(svn_authz_t **authz_p, svn_stream_t *stream,
- svn_stream_t *groups_stream, apr_pool_t *pool)
+ svn_stream_t *groups_stream, apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool)
{
- svn_authz_t *authz = apr_palloc(pool, sizeof(*authz));
+ svn_authz_t *authz = apr_palloc(result_pool, sizeof(*authz));
/* Parse the authz stream */
- SVN_ERR(svn_config_parse(&authz->cfg, stream, TRUE, TRUE, pool));
+ SVN_ERR(svn_config_parse(&authz->cfg, stream, TRUE, TRUE,
+ result_pool, scratch_pool));
if (groups_stream)
{
@@ -1005,13 +1012,14 @@ svn_repos_authz_parse(svn_authz_t **authz_p, svn_s
svn_config_t *groups_cfg;
/* Parse the groups stream */
- SVN_ERR(svn_config_parse(&groups_cfg, groups_stream, TRUE, TRUE, pool));
+ SVN_ERR(svn_config_parse(&groups_cfg, groups_stream, TRUE, TRUE,
+ result_pool, scratch_pool));
- SVN_ERR(authz_copy_groups(authz, groups_cfg, pool));
+ SVN_ERR(authz_copy_groups(authz, groups_cfg, scratch_pool));
}
/* Make sure there are no errors in the configuration. */
- SVN_ERR(authz_validate(authz, pool));
+ SVN_ERR(authz_validate(authz, scratch_pool));
*authz_p = authz;
return SVN_NO_ERROR;
Index: subversion/libsvn_repos/deprecated.c
===================================================================
--- subversion/libsvn_repos/deprecated.c (revision 1477539)
+++ subversion/libsvn_repos/deprecated.c (working copy)
@@ -30,6 +30,7 @@
#include "svn_compat.h"
#include "svn_hash.h"
#include "svn_props.h"
+#include "svn_pools.h"
#include "svn_private_config.h"
@@ -1012,6 +1013,10 @@ svn_error_t *
svn_repos_authz_read(svn_authz_t **authz_p, const char *file,
svn_boolean_t must_exist, apr_pool_t *pool)
{
+ /* The lower layers still create subpools of result_pool to control
+ memory use, if that changes we will need to create a subpool
+ here. */
return svn_repos__authz_read(authz_p, file, NULL, must_exist,
- FALSE, pool);
+ FALSE, pool, pool);
+
}
Index: subversion/libsvn_repos/hooks.c
===================================================================
--- subversion/libsvn_repos/hooks.c (revision 1477539)
+++ subversion/libsvn_repos/hooks.c (working copy)
@@ -422,7 +422,7 @@ svn_repos__parse_hooks_env(apr_hash_t **hooks_env_
if (local_abspath)
{
SVN_ERR(svn_config_read3(&cfg, local_abspath, FALSE,
- TRUE, TRUE, scratch_pool));
+ TRUE, TRUE, scratch_pool, scratch_pool));
b.cfg = cfg;
b.hooks_env = apr_hash_make(result_pool);
(void)svn_config_enumerate_sections2(cfg, parse_hooks_env_section, &b,
Index: subversion/libsvn_repos/repos.h
===================================================================
--- subversion/libsvn_repos/repos.h (revision 1477539)
+++ subversion/libsvn_repos/repos.h (working copy)
@@ -380,7 +380,8 @@ svn_repos__authz_read(svn_authz_t **authz_p,
const char *groups_path,
svn_boolean_t must_exist,
svn_boolean_t accept_urls,
- apr_pool_t *pool);
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool);
/*** Utility Functions ***/
Index: subversion/libsvn_subr/config.c
===================================================================
--- subversion/libsvn_subr/config.c (revision 1477539)
+++ subversion/libsvn_subr/config.c (working copy)
@@ -102,7 +102,8 @@ svn_config_read3(svn_config_t **cfgp, const char *
svn_boolean_t must_exist,
svn_boolean_t section_names_case_sensitive,
svn_boolean_t option_names_case_sensitive,
- apr_pool_t *result_pool)
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool)
{
svn_config_t *cfg;
svn_error_t *err;
@@ -120,10 +121,10 @@ svn_config_read3(svn_config_t **cfgp, const char *
#ifdef WIN32
if (0 == strncmp(file, SVN_REGISTRY_PREFIX, SVN_REGISTRY_PREFIX_LEN))
err = svn_config__parse_registry(cfg, file + SVN_REGISTRY_PREFIX_LEN,
- must_exist, result_pool);
+ must_exist, scratch_pool);
else
#endif /* WIN32 */
- err = svn_config__parse_file(cfg, file, must_exist, result_pool);
+ err = svn_config__parse_file(cfg, file, must_exist, scratch_pool);
if (err != SVN_NO_ERROR)
return err;
@@ -137,26 +138,21 @@ svn_error_t *
svn_config_parse(svn_config_t **cfgp, svn_stream_t *stream,
svn_boolean_t section_names_case_sensitive,
svn_boolean_t option_names_case_sensitive,
- apr_pool_t *result_pool)
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool)
{
svn_config_t *cfg;
- svn_error_t *err;
- apr_pool_t *scratch_pool = svn_pool_create(result_pool);
- err = svn_config_create2(&cfg,
- section_names_case_sensitive,
- option_names_case_sensitive,
- result_pool);
+ SVN_ERR(svn_config_create2(&cfg,
+ section_names_case_sensitive,
+ option_names_case_sensitive,
+ result_pool));
- if (err == SVN_NO_ERROR)
- err = svn_config__parse_stream(cfg, stream, result_pool, scratch_pool);
+ SVN_ERR(svn_config__parse_stream(cfg, stream, scratch_pool));
- if (err == SVN_NO_ERROR)
- *cfgp = cfg;
+ *cfgp = cfg;
- svn_pool_destroy(scratch_pool);
-
- return err;
+ return SVN_NO_ERROR;
}
/* Read various configuration sources into *CFGP, in this order, with
@@ -181,6 +177,10 @@ read_all(svn_config_t **cfgp,
const char *usr_file_path,
apr_pool_t *pool)
{
+ /* In the past svn_config_read2 used a subpool internally, now
+ svn_config_read3 requires the caller to pass a scratch pool. */
+ apr_pool_t *scratch_pool = svn_pool_create(pool);
+
svn_boolean_t red_config = FALSE; /* "red" is the past tense of "read" */
/*** Read system-wide configurations first... ***/
@@ -189,7 +189,7 @@ read_all(svn_config_t **cfgp,
if (sys_registry_path)
{
SVN_ERR(svn_config_read3(cfgp, sys_registry_path, FALSE, FALSE, FALSE,
- pool));
+ pool, scratch_pool));
red_config = TRUE;
}
#endif /* WIN32 */
@@ -200,8 +200,8 @@ read_all(svn_config_t **cfgp,
SVN_ERR(svn_config_merge(*cfgp, sys_file_path, FALSE));
else
{
- SVN_ERR(svn_config_read3(cfgp, sys_file_path,
- FALSE, FALSE, FALSE, pool));
+ SVN_ERR(svn_config_read3(cfgp, sys_file_path, FALSE, FALSE, FALSE,
+ pool, scratch_pool));
red_config = TRUE;
}
}
@@ -216,7 +216,8 @@ read_all(svn_config_t **cfgp,
else
{
SVN_ERR(svn_config_read3(cfgp, usr_registry_path,
- FALSE, FALSE, FALSE, pool));
+ FALSE, FALSE, FALSE,
+ pool, scratch_pool));
red_config = TRUE;
}
}
@@ -229,7 +230,8 @@ read_all(svn_config_t **cfgp,
else
{
SVN_ERR(svn_config_read3(cfgp, usr_file_path,
- FALSE, FALSE, FALSE, pool));
+ FALSE, FALSE, FALSE,
+ pool, scratch_pool));
red_config = TRUE;
}
}
@@ -237,6 +239,8 @@ read_all(svn_config_t **cfgp,
if (! red_config)
SVN_ERR(svn_config_create2(cfgp, FALSE, FALSE, pool));
+ svn_pool_destroy(scratch_pool);
+
return SVN_NO_ERROR;
}
@@ -368,7 +372,7 @@ svn_config_merge(svn_config_t *cfg, const char *fi
SVN_ERR(svn_config_read3(&merge_cfg, file, must_exist,
cfg->section_names_case_sensitive,
cfg->option_names_case_sensitive,
- cfg->pool));
+ cfg->pool, cfg->pool));
/* Now copy the new options into the original table. */
for_each_option(merge_cfg, cfg, merge_cfg->pool, merge_callback);
Index: subversion/libsvn_subr/config_file.c
===================================================================
--- subversion/libsvn_subr/config_file.c (revision 1477539)
+++ subversion/libsvn_subr/config_file.c (working copy)
@@ -398,24 +398,23 @@ svn_config__sys_config_path(const char **path_p,
svn_error_t *
svn_config__parse_file(svn_config_t *cfg, const char *file,
- svn_boolean_t must_exist, apr_pool_t *result_pool)
+ svn_boolean_t must_exist, apr_pool_t *scratch_pool)
{
svn_error_t *err = SVN_NO_ERROR;
svn_stream_t *stream;
- apr_pool_t *scratch_pool = svn_pool_create(result_pool);
+ apr_pool_t *subpool = svn_pool_create(scratch_pool);
- err = svn_stream_open_readonly(&stream, file, scratch_pool, scratch_pool);
+ err = svn_stream_open_readonly(&stream, file, scratch_pool, subpool);
if (! must_exist && err && APR_STATUS_IS_ENOENT(err->apr_err))
{
svn_error_clear(err);
- svn_pool_destroy(scratch_pool);
return SVN_NO_ERROR;
}
else
SVN_ERR(err);
- err = svn_config__parse_stream(cfg, stream, result_pool, scratch_pool);
+ err = svn_config__parse_stream(cfg, stream, subpool);
if (err != SVN_NO_ERROR)
{
@@ -422,11 +421,12 @@ svn_config__parse_file(svn_config_t *cfg, const ch
/* Add the filename to the error stack. */
err = svn_error_createf(err->apr_err, err,
"Error while parsing config file: %s:",
- svn_dirent_local_style(file, scratch_pool));
+ svn_dirent_local_style(file, subpool));
}
- /* Close the streams (and other cleanup): */
- svn_pool_destroy(scratch_pool);
+ /* Close the streams (and other cleanup).
+ ### Perhaps an explicitly svn_stream close instead of a subpool? */
+ svn_pool_destroy(subpool);
return err;
}
@@ -433,7 +433,7 @@ svn_config__parse_file(svn_config_t *cfg, const ch
svn_error_t *
svn_config__parse_stream(svn_config_t *cfg, svn_stream_t *stream,
- apr_pool_t *result_pool, apr_pool_t *scratch_pool)
+ apr_pool_t *scratch_pool)
{
parse_context_t *ctx;
int ch, count;
Index: subversion/libsvn_subr/config_impl.h
===================================================================
--- subversion/libsvn_subr/config_impl.h (revision 1477539)
+++ subversion/libsvn_subr/config_impl.h (working copy)
@@ -77,12 +77,11 @@ struct svn_config_t
svn_error_t *svn_config__parse_file(svn_config_t *cfg,
const char *file,
svn_boolean_t must_exist,
- apr_pool_t *pool);
+ apr_pool_t *scratch_pool);
/* Read sections and options from a stream. */
svn_error_t *svn_config__parse_stream(svn_config_t *cfg,
svn_stream_t *stream,
- apr_pool_t *result_pool,
apr_pool_t *scratch_pool);
/* The name of the magic [DEFAULT] section. */
@@ -99,7 +98,7 @@ svn_error_t *svn_config__win_config_path(const cha
svn_error_t *svn_config__parse_registry(svn_config_t *cfg,
const char *file,
svn_boolean_t must_exist,
- apr_pool_t *pool);
+ apr_pool_t *scratch_pool);
/* ### It's unclear to me whether this registry stuff should get the
double underscore or not, and if so, where the extra underscore
Index: subversion/libsvn_subr/config_win.c
===================================================================
--- subversion/libsvn_subr/config_win.c (revision 1477539)
+++ subversion/libsvn_subr/config_win.c (working copy)
@@ -154,9 +154,8 @@ parse_section(svn_config_t *cfg, HKEY hkey, const
svn_error_t *
svn_config__parse_registry(svn_config_t *cfg, const char *file,
- svn_boolean_t must_exist, apr_pool_t *pool)
+ svn_boolean_t must_exist, apr_pool_t *scratch_pool)
{
- apr_pool_t *subpool;
svn_stringbuf_t *section, *option, *value;
svn_error_t *svn_err = SVN_NO_ERROR;
HKEY base_hkey, hkey;
@@ -177,7 +176,7 @@ svn_config__parse_registry(svn_config_t *cfg, cons
{
return svn_error_createf(SVN_ERR_BAD_FILENAME, NULL,
"Unrecognised registry path '%s'",
- svn_dirent_local_style(file, pool));
+ svn_dirent_local_style(file, scratch_pool));
}
err = RegOpenKeyEx(base_hkey, file, 0,
@@ -189,20 +188,19 @@ svn_config__parse_registry(svn_config_t *cfg, cons
if (!is_enoent)
return svn_error_createf(SVN_ERR_BAD_FILENAME, NULL,
"Can't open registry key '%s'",
- svn_dirent_local_style(file, pool));
+ svn_dirent_local_style(file, scratch_pool));
else if (must_exist && is_enoent)
return svn_error_createf(SVN_ERR_BAD_FILENAME, NULL,
"Can't find registry key '%s'",
- svn_dirent_local_style(file, pool));
+ svn_dirent_local_style(file, scratch_pool));
else
return SVN_NO_ERROR;
}
- subpool = svn_pool_create(pool);
- section = svn_stringbuf_create_empty(subpool);
- option = svn_stringbuf_create_empty(subpool);
- value = svn_stringbuf_create_empty(subpool);
+ section = svn_stringbuf_create_empty(scratch_pool);
+ option = svn_stringbuf_create_empty(scratch_pool);
+ value = svn_stringbuf_create_empty(scratch_pool);
/* The top-level values belong to the [DEFAULT] section */
svn_err = parse_section(cfg, hkey, SVN_CONFIG__DEFAULT_SECTION,
@@ -252,7 +250,6 @@ svn_config__parse_registry(svn_config_t *cfg, cons
cleanup:
RegCloseKey(hkey);
- svn_pool_destroy(subpool);
return svn_err;
}
Index: subversion/libsvn_subr/deprecated.c
===================================================================
--- subversion/libsvn_subr/deprecated.c (revision 1477539)
+++ subversion/libsvn_subr/deprecated.c (working copy)
@@ -1193,11 +1193,14 @@ svn_config_read2(svn_config_t **cfgp, const char *
svn_boolean_t section_names_case_sensitive,
apr_pool_t *result_pool)
{
+ /* The lower layers still create subpools of result_pool to control
+ memory use, if that changes we will need to create a subpool
+ here. */
return svn_error_trace(svn_config_read3(cfgp, file,
must_exist,
section_names_case_sensitive,
FALSE,
- result_pool));
+ result_pool, result_pool));
}
svn_error_t *
@@ -1205,9 +1208,9 @@ svn_config_read(svn_config_t **cfgp, const char *f
svn_boolean_t must_exist,
apr_pool_t *result_pool)
{
- return svn_error_trace(svn_config_read3(cfgp, file,
+ return svn_error_trace(svn_config_read2(cfgp, file,
must_exist,
- FALSE, FALSE,
+ FALSE,
result_pool));
}
Index: subversion/mod_authz_svn/mod_authz_svn.c
===================================================================
--- subversion/mod_authz_svn/mod_authz_svn.c (revision 1477539)
+++ subversion/mod_authz_svn/mod_authz_svn.c (working copy)
@@ -412,7 +412,7 @@ get_access_conf(request_rec *r, authz_svn_config_r
svn_err = svn_repos_authz_read2(&access_conf, access_file,
groups_file, TRUE,
- r->connection->pool);
+ r->connection->pool, scratch_pool);
if (svn_err)
{
Index: subversion/svnserve/serve.c
===================================================================
--- subversion/svnserve/serve.c (revision 1477539)
+++ subversion/svnserve/serve.c (working copy)
@@ -277,7 +277,7 @@ svn_error_t *load_pwdb_config(server_baton_t *serv
pwdb_path = svn_dirent_join(server->base, pwdb_path, pool);
err = svn_config_read3(&server->pwdb, pwdb_path, TRUE,
- FALSE, FALSE, pool);
+ FALSE, FALSE, pool, pool);
if (err)
{
log_server_error(err, server, conn, pool);
@@ -371,7 +371,7 @@ svn_error_t *load_authz_config(server_baton_t *ser
if (!err)
err = svn_repos_authz_read2(&server->authzdb, authzdb_path,
- groupsdb_path, TRUE, pool);
+ groupsdb_path, TRUE, pool, pool);
if (err)
{
@@ -3293,7 +3293,7 @@ static svn_error_t *find_repos(const char *url, co
FALSE, /* must_exist */
FALSE, /* section_names_case_sensitive */
FALSE, /* option_names_case_sensitive */
- pool));
+ pool, pool));
SVN_ERR(load_pwdb_config(b, conn, pool));
SVN_ERR(load_authz_config(b, conn, repos_root, pool));
}
Index: subversion/svnserve/svnserve.c
===================================================================
--- subversion/svnserve/svnserve.c (revision 1477539)
+++ subversion/svnserve/svnserve.c (working copy)
@@ -759,7 +759,7 @@ int main(int argc, const char *argv[])
TRUE, /* must_exist */
FALSE, /* section_names_case_sensitive */
FALSE, /* option_names_case_sensitive */
- pool));
+ pool, pool));
}
if (log_filename)
Index: subversion/tests/libsvn_repos/repos-test.c
===================================================================
--- subversion/tests/libsvn_repos/repos-test.c (revision 1477539)
+++ subversion/tests/libsvn_repos/repos-test.c (working copy)
@@ -1159,7 +1159,7 @@ authz_get_handle(svn_authz_t **authz_p, const char
SVN_ERR_W(svn_stream_puts(stream, authz_contents),
"Writing authz contents to stream");
- SVN_ERR_W(svn_repos_authz_parse(authz_p, stream, NULL, pool),
+ SVN_ERR_W(svn_repos_authz_parse(authz_p, stream, NULL, pool, pool),
"Parsing the authz contents");
SVN_ERR_W(svn_stream_close(stream),
@@ -1458,15 +1458,16 @@ in_repo_authz(const svn_test_opts_t *opts,
noent_authz_url = apr_pstrcat(pool, repos_url, "/A/authz", (char *)NULL);
/* absolute file URL. */
- SVN_ERR(svn_repos_authz_read2(&authz_cfg, authz_url, NULL, TRUE, pool));
+ SVN_ERR(svn_repos_authz_read2(&authz_cfg, authz_url, NULL, TRUE, pool, pool));
SVN_ERR(authz_check_access(authz_cfg, test_set, pool));
/* Non-existant path in the repo with must_exist set to FALSE */
SVN_ERR(svn_repos_authz_read2(&authz_cfg, noent_authz_url, NULL,
- FALSE, pool));
+ FALSE, pool, pool));
/* Non-existant path in the repo with must_exist set to TRUE */
- err = svn_repos_authz_read2(&authz_cfg, noent_authz_url, NULL, TRUE, pool);
+ err = svn_repos_authz_read2(&authz_cfg, noent_authz_url, NULL, TRUE,
+ pool, pool);
if (!err || err->apr_err != SVN_ERR_ILLEGAL_TARGET)
return svn_error_createf(SVN_ERR_TEST_FAILED, err,
"Got %s error instead of expected "
@@ -1476,7 +1477,7 @@ in_repo_authz(const svn_test_opts_t *opts,
/* http:// URL which is unsupported */
err = svn_repos_authz_read2(&authz_cfg, "http://example.com/repo/authz",
- NULL, TRUE, pool);
+ NULL, TRUE, pool, pool);
if (!err || err->apr_err != SVN_ERR_RA_ILLEGAL_URL)
return svn_error_createf(SVN_ERR_TEST_FAILED, err,
"Got %s error instead of expected "
@@ -1486,7 +1487,7 @@ in_repo_authz(const svn_test_opts_t *opts,
/* svn:// URL which is unsupported */
err = svn_repos_authz_read2(&authz_cfg, "svn://example.com/repo/authz",
- NULL, TRUE, pool);
+ NULL, TRUE, pool, pool);
if (!err || err->apr_err != SVN_ERR_RA_ILLEGAL_URL)
return svn_error_createf(SVN_ERR_TEST_FAILED, err,
"Got %s error instead of expected "
@@ -1601,23 +1602,24 @@ in_repo_groups_authz(const svn_test_opts_t *opts,
/* absolute file URLs. */
- SVN_ERR(svn_repos_authz_read2(&authz_cfg, authz_url, groups_url, TRUE, pool));
+ SVN_ERR(svn_repos_authz_read2(&authz_cfg, authz_url, groups_url, TRUE,
+ pool, pool));
SVN_ERR(authz_check_access(authz_cfg, test_set, pool));
/* Non-existent path for the groups file with must_exist
* set to TRUE */
SVN_ERR(svn_repos_authz_read2(&authz_cfg, empty_authz_url, noent_groups_url,
- FALSE, pool));
+ FALSE, pool, pool));
/* Non-existent paths for both the authz and the groups files
* with must_exist set to TRUE */
SVN_ERR(svn_repos_authz_read2(&authz_cfg, noent_authz_url, noent_groups_url,
- FALSE, pool));
+ FALSE, pool, pool));
/* Non-existent path for the groups file with must_exist
* set to TRUE */
err = svn_repos_authz_read2(&authz_cfg, empty_authz_url, noent_groups_url,
- TRUE, pool);
+ TRUE, pool, pool);
if (!err || err->apr_err != SVN_ERR_ILLEGAL_TARGET)
return svn_error_createf(SVN_ERR_TEST_FAILED, err,
"Got %s error instead of expected "
@@ -1628,7 +1630,7 @@ in_repo_groups_authz(const svn_test_opts_t *opts,
/* http:// URL which is unsupported */
err = svn_repos_authz_read2(&authz_cfg, empty_authz_url,
"http://example.com/repo/groups",
- TRUE, pool);
+ TRUE, pool, pool);
if (!err || err->apr_err != SVN_ERR_RA_ILLEGAL_URL)
return svn_error_createf(SVN_ERR_TEST_FAILED, err,
"Got %s error instead of expected "
@@ -1639,7 +1641,7 @@ in_repo_groups_authz(const svn_test_opts_t *opts,
/* svn:// URL which is unsupported */
err = svn_repos_authz_read2(&authz_cfg, empty_authz_url,
"http://example.com/repo/groups",
- TRUE, pool);
+ TRUE, pool, pool);
if (!err || err->apr_err != SVN_ERR_RA_ILLEGAL_URL)
return svn_error_createf(SVN_ERR_TEST_FAILED, err,
"Got %s error instead of expected "
@@ -1683,7 +1685,7 @@ authz_groups_get_handle(svn_authz_t **authz_p,
/* Read the authz configuration back and start testing. */
SVN_ERR_W(svn_repos_authz_read2(authz_p, authz_file_path,
- groups_file_path, TRUE, pool),
+ groups_file_path, TRUE, pool, pool),
"Opening test authz and groups files");
/* Done with the files. */
@@ -1707,7 +1709,8 @@ authz_groups_get_handle(svn_authz_t **authz_p,
"Writing groups contents to stream");
/* Read the authz configuration from the streams and start testing. */
- SVN_ERR_W(svn_repos_authz_parse(authz_p, stream, groups_stream, pool),
+ SVN_ERR_W(svn_repos_authz_parse(authz_p, stream, groups_stream,
+ pool, pool),
"Parsing the authz and groups contents");
/* Done with the streams. */
Index: subversion/tests/libsvn_subr/cache-test.c
===================================================================
--- subversion/tests/libsvn_subr/cache-test.c (revision 1477539)
+++ subversion/tests/libsvn_subr/cache-test.c (working copy)
@@ -156,7 +156,7 @@ test_memcache_basic(const svn_test_opts_t *opts,
if (opts->config_file)
{
SVN_ERR(svn_config_read3(&config, opts->config_file,
- TRUE, FALSE, FALSE, pool));
+ TRUE, FALSE, FALSE, pool, pool));
SVN_ERR(svn_cache__make_memcache_from_config(&memcache, config, pool));
}
@@ -224,7 +224,7 @@ test_memcache_long_key(const svn_test_opts_t *opts
if (opts->config_file)
{
SVN_ERR(svn_config_read3(&config, opts->config_file,
- TRUE, FALSE, FALSE, pool));
+ TRUE, FALSE, FALSE, pool, pool));
SVN_ERR(svn_cache__make_memcache_from_config(&memcache, config, pool));
}
Index: subversion/tests/libsvn_subr/config-test.c
===================================================================
--- subversion/tests/libsvn_subr/config-test.c (revision 1477539)
+++ subversion/tests/libsvn_subr/config-test.c (working copy)
@@ -109,7 +109,7 @@ test_text_retrieval(apr_pool_t *pool)
SVN_ERR(init_params(pool));
cfg_file = apr_pstrcat(pool, srcdir, "/", "config-test.cfg", (char *)NULL);
- SVN_ERR(svn_config_read3(&cfg, cfg_file, TRUE, FALSE, FALSE, pool));
+ SVN_ERR(svn_config_read3(&cfg, cfg_file, TRUE, FALSE, FALSE, pool, pool));
/* Test values retrieved from our ConfigParser instance against
values retrieved using svn_config. */
@@ -160,7 +160,7 @@ test_boolean_retrieval(apr_pool_t *pool)
SVN_ERR(init_params(pool));
cfg_file = apr_pstrcat(pool, srcdir, "/", "config-test.cfg", (char *)NULL);
- SVN_ERR(svn_config_read3(&cfg, cfg_file, TRUE, FALSE, FALSE, pool));
+ SVN_ERR(svn_config_read3(&cfg, cfg_file, TRUE, FALSE, FALSE, pool, pool));
for (i = 0; true_keys[i] != NULL; i++)
{
@@ -220,7 +220,7 @@ test_has_section_case_insensitive(apr_pool_t *pool
SVN_ERR(init_params(pool));
cfg_file = apr_pstrcat(pool, srcdir, "/", "config-test.cfg", (char *)NULL);
- SVN_ERR(svn_config_read3(&cfg, cfg_file, TRUE, FALSE, FALSE, pool));
+ SVN_ERR(svn_config_read3(&cfg, cfg_file, TRUE, FALSE, FALSE, pool, pool));
if (! svn_config_has_section(cfg, "section1"))
return fail(pool, "Failed to find section1");
@@ -250,7 +250,7 @@ test_has_section_case_sensitive(apr_pool_t *pool)
SVN_ERR(init_params(pool));
cfg_file = apr_pstrcat(pool, srcdir, "/", "config-test.cfg", (char *)NULL);
- SVN_ERR(svn_config_read3(&cfg, cfg_file, TRUE, TRUE, FALSE, pool));
+ SVN_ERR(svn_config_read3(&cfg, cfg_file, TRUE, TRUE, FALSE, pool, pool));
if (! svn_config_has_section(cfg, "section1"))
return fail(pool, "Failed to find section1");
@@ -293,7 +293,7 @@ test_has_option_case_sensitive(apr_pool_t *pool)
SVN_ERR(init_params(pool));
cfg_file = apr_pstrcat(pool, srcdir, "/", "config-test.cfg", (char *)NULL);
- SVN_ERR(svn_config_read3(&cfg, cfg_file, TRUE, TRUE, TRUE, pool));
+ SVN_ERR(svn_config_read3(&cfg, cfg_file, TRUE, TRUE, TRUE, pool, pool));
for (i = 0; i < test_data_size; ++i)
{
@@ -325,7 +325,7 @@ test_stream_interface(apr_pool_t *pool)
cfg_file = apr_pstrcat(pool, srcdir, "/", "config-test.cfg", (char *)NULL);
SVN_ERR(svn_stream_open_readonly(&stream, cfg_file, pool, pool));
- SVN_ERR(svn_config_parse(&cfg, stream, TRUE, TRUE, pool));
+ SVN_ERR(svn_config_parse(&cfg, stream, TRUE, TRUE, pool, pool));
/* nominal test to make sure cfg is populated with something since
* svn_config_parse will happily return an empty cfg if the stream is
Index: tools/server-side/mod_dontdothat/mod_dontdothat.c
===================================================================
--- tools/server-side/mod_dontdothat/mod_dontdothat.c (revision 1477539)
+++ tools/server-side/mod_dontdothat/mod_dontdothat.c (working copy)
@@ -585,7 +585,7 @@ dontdothat_insert_filters(request_rec *r)
/* XXX is there a way to error out from this point? Would be nice... */
err = svn_config_read3(&config, cfg->config_file, TRUE,
- FALSE, TRUE, r->pool);
+ FALSE, TRUE, r->pool, r->pool);
if (err)
{
char buff[256];
Index: tools/server-side/svnauthz.c
===================================================================
--- tools/server-side/svnauthz.c (revision 1477539)
+++ tools/server-side/svnauthz.c (working copy)
@@ -239,7 +239,8 @@ get_authz_from_txn(svn_authz_t **authz, const char
else
groups_contents = NULL;
- err = svn_repos_authz_parse(authz, authz_contents, groups_contents, pool);
+ err = svn_repos_authz_parse(authz, authz_contents, groups_contents,
+ pool, pool);
/* Add the filename to the error stack since the parser doesn't have it. */
if (err != SVN_NO_ERROR)
@@ -268,7 +269,7 @@ get_authz(svn_authz_t **authz, struct svnauthz_opt
/* Else */
return svn_repos_authz_read2(authz, opt_state->authz_file,
opt_state->groups_file,
- TRUE, pool);
+ TRUE, pool, pool);
}
static svn_error_t *
--
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download
Received on 2013-04-30 19:39:39 CEST