You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by br...@apache.org on 2013/01/01 00:12:43 UTC

svn commit: r1427254 - /subversion/trunk/tools/server-side/svnauthz-validate.c

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.

* 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.

Re: svn commit: r1427254 - /subversion/trunk/tools/server-side/svnauthz-validate.c

Posted by Daniel Shahaf <da...@elego.de>.
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 Daniel Shahaf <da...@elego.de>.
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;
> +}
> 
>