You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by Daniel Shahaf <da...@elego.de> on 2013/01/03 21:17:22 UTC
Re: svn commit: r1427254 -
/subversion/trunk/tools/server-side/svnauthz-validate.c
breser@apache.org wrote on Mon, Dec 31, 2012 at 23:12:43 -0000:
> Author: breser
> Date: Mon Dec 31 23:12:42 2012
> New Revision: 1427254
>
> URL: http://svn.apache.org/viewvc?rev=1427254&view=rev
> Log:
> svnauthz-validate: Split main in main and sub_main so pool is cleaned up
> properly.
>
What's the benefit of this? Destroying the pool at process exit ---
it's not to reclaim memory. Is it to ensure some pool cleanup callback
fires?
> * tools/server-side/svnauthz-validate.c
> (sub_main): Rename old main to sub_main.
> (main): Move the svn_cmdline_init() and pool creation/destruction in new
> function here.
>
> Modified:
> subversion/trunk/tools/server-side/svnauthz-validate.c
>
> Modified: subversion/trunk/tools/server-side/svnauthz-validate.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/tools/server-side/svnauthz-validate.c?rev=1427254&r1=1427253&r2=1427254&view=diff
> ==============================================================================
> --- subversion/trunk/tools/server-side/svnauthz-validate.c (original)
> +++ subversion/trunk/tools/server-side/svnauthz-validate.c Mon Dec 31 23:12:42 2012
> @@ -105,10 +105,9 @@ get_authz_from_txn(svn_authz_t **authz,
> return SVN_NO_ERROR;
> }
>
> -int
> -main(int argc, const char **argv)
> +static int
> +sub_main(int argc, const char *argv[], apr_pool_t *pool)
> {
> - apr_pool_t *pool;
> svn_error_t *err;
> apr_status_t apr_err;
> svn_authz_t *authz;
> @@ -132,12 +131,6 @@ main(int argc, const char **argv)
> opts.username = opts.fspath = opts.repos_name = opts.txn = NULL;
> opts.repos_path = NULL;
>
> - /* Initialize the app. Send all error messages to 'stderr'. */
> - if (svn_cmdline_init(argv[0], stderr) != EXIT_SUCCESS)
> - return 2;
> -
> - pool = svn_pool_create(NULL);
> -
> /* Repeat svn_cmdline__getopt_init() inline. */
> apr_err = apr_getopt_init(&os, pool, argc, argv);
> if (apr_err)
> @@ -282,8 +275,6 @@ main(int argc, const char **argv)
> );
> }
>
> - svn_pool_destroy(pool);
> -
> if (err)
> {
> svn_handle_error2(err, stderr, FALSE, "svnauthz-validate: ");
> @@ -295,3 +286,21 @@ main(int argc, const char **argv)
> return 0;
> }
> }
> +
> +int
> +main(int argc, const char *argv[])
> +{
> + apr_pool_t *pool;
> + int exit_code;
> +
> + /* Initialize the app. Send all error messages to 'stderr'. */
> + if (svn_cmdline_init(argv[0], stderr) != EXIT_SUCCESS)
> + return 2;
> +
> + pool = svn_pool_create(NULL);
> +
> + exit_code = sub_main(argc, argv, pool);
> +
> + svn_pool_destroy(pool);
> + return exit_code;
> +}
>
>
Re: svn commit: r1427254 - /subversion/trunk/tools/server-side/svnauthz-validate.c
Posted by Ben Reser <be...@reser.org>.
On Thu, Jan 3, 2013 at 12:40 PM, Branko Čibej <br...@wandisco.com> wrote:
> We have maintainer-mode magic that aborts if it detects an svn_error_t
> that was not properly cleaned up.
That's exactly what it was, we didn't call svn_error_clear() after
svn_handle_error2().
Re: svn commit: r1427254 - /subversion/trunk/tools/server-side/svnauthz-validate.c
Posted by Branko Čibej <br...@wandisco.com>.
On 03.01.2013 21:38, Ben Reser wrote:
> On Thu, Jan 3, 2013 at 12:17 PM, Daniel Shahaf <da...@elego.de> wrote:
>> What's the benefit of this? Destroying the pool at process exit ---
>> it's not to reclaim memory. Is it to ensure some pool cleanup callback
>> fires?
> It's mostly just a correctness thing. I noticed it when I noticed
> that 1.7.x's svnauthz-validate crashes on error handling.
We have maintainer-mode magic that aborts if it detects an svn_error_t
that was not properly cleaned up.
-- Brane
--
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com
Re: svn commit: r1427254 - /subversion/trunk/tools/server-side/svnauthz-validate.c
Posted by Ben Reser <be...@reser.org>.
On Thu, Jan 3, 2013 at 12:17 PM, Daniel Shahaf <da...@elego.de> wrote:
> What's the benefit of this? Destroying the pool at process exit ---
> it's not to reclaim memory. Is it to ensure some pool cleanup callback
> fires?
It's mostly just a correctness thing. I noticed it when I noticed
that 1.7.x's svnauthz-validate crashes on error handling.
Right now it has no impact whatsoever since we're doing only read-only
ops on a repository. But if we weren't then yes the pool cleanups
might be an issue.
Re: svn commit: r1427254 - /subversion/trunk/tools/server-side/svnauthz-validate.c
Posted by Ben Reser <be...@reser.org>.
On Thu, Jan 3, 2013 at 12:17 PM, Daniel Shahaf <da...@elego.de> wrote:
> What's the benefit of this? Destroying the pool at process exit ---
> it's not to reclaim memory. Is it to ensure some pool cleanup callback
> fires?
It's mostly just a correctness thing. I noticed it when I noticed
that 1.7.x's svnauthz-validate crashes on error handling.
Right now it has no impact whatsoever since we're doing only read-only
ops on a repository. But if we weren't then yes the pool cleanups
might be an issue.