You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ju...@apache.org on 2013/11/21 13:50:50 UTC

svn commit: r1544162 - in /subversion/trunk/subversion: svnmucc/svnmucc.c svnserve/svnserve.c svnversion/svnversion.c

Author: julianfoad
Date: Thu Nov 21 12:50:49 2013
New Revision: 1544162

URL: http://svn.apache.org/r1544162
Log:
Remnove most of the direct calls to exit() in the command-line tools, in
favour of returning through main(), so that consistent kinds of clean-up can
take place. As a result, any error in flushing stdout will now be reported
in these cases like in all other cases.

* subversion/svnmucc/svnmucc.c
  (handle_error): Delete.
  (usage): Don't exit. Take the output stream directly as a parameter
    instead of deducing it from an exit-code parameter.
  (insufficient): Don't exit, just return the error.
  (sub_main): Don't exit directly or expect handle_error(), usage() or
    insufficient() to do so; instead, return appropriately.

* subversion/svnserve/svnserve.c,
  subversion/svnversion/svnversion.c
  (usage): Don't exit.
  (help): Don't exit.
  (sub_main): Don't exit directly or expect usage() or help() to do so;
    instead, return appropriately.

Modified:
    subversion/trunk/subversion/svnmucc/svnmucc.c
    subversion/trunk/subversion/svnserve/svnserve.c
    subversion/trunk/subversion/svnversion/svnversion.c

Modified: subversion/trunk/subversion/svnmucc/svnmucc.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnmucc/svnmucc.c?rev=1544162&r1=1544161&r2=1544162&view=diff
==============================================================================
--- subversion/trunk/subversion/svnmucc/svnmucc.c (original)
+++ subversion/trunk/subversion/svnmucc/svnmucc.c Thu Nov 21 12:50:49 2013
@@ -59,16 +59,6 @@
 #include "private/svn_ra_private.h"
 #include "private/svn_string_private.h"
 
-static void handle_error(svn_error_t *err, apr_pool_t *pool)
-{
-  if (err)
-    svn_handle_error2(err, stderr, FALSE, "svnmucc: ");
-  svn_error_clear(err);
-  if (pool)
-    svn_pool_destroy(pool);
-  exit(EXIT_FAILURE);
-}
-
 /* Version compatibility check */
 static svn_error_t *
 check_lib_versions(void)
@@ -911,10 +901,10 @@ sanitize_url(const char *url,
   return svn_uri_canonicalize(url, pool);
 }
 
+/* Print a usage message on STREAM. */
 static void
-usage(apr_pool_t *pool, int exit_val)
+usage(FILE *stream, apr_pool_t *pool)
 {
-  FILE *stream = exit_val == EXIT_SUCCESS ? stdout : stderr;
   svn_error_clear(svn_cmdline_fputs(
     _("usage: svnmucc ACTION...\n"
       "Subversion multiple URL command client.\n"
@@ -958,16 +948,13 @@ usage(apr_pool_t *pool, int exit_val)
       "  --no-auth-cache        : do not cache authentication tokens\n"
       "  --version              : print version information\n"),
                   stream, pool));
-  svn_pool_destroy(pool);
-  exit(exit_val);
 }
 
-static void
-insufficient(apr_pool_t *pool)
+static svn_error_t *
+insufficient(void)
 {
-  handle_error(svn_error_create(SVN_ERR_INCORRECT_PARAMS, NULL,
-                                "insufficient arguments"),
-               pool);
+  return svn_error_create(SVN_ERR_INCORRECT_PARAMS, NULL,
+                          "insufficient arguments");
 }
 
 static svn_error_t *
@@ -1110,7 +1097,7 @@ sub_main(int *exit_code, int argc, const
       if (APR_STATUS_IS_EOF(status))
         break;
       if (status != APR_SUCCESS)
-        handle_error(svn_error_wrap_apr(status, "getopt failure"), pool);
+        return svn_error_wrap_apr(status, "getopt failure");
       switch(opt)
         {
         case 'm':
@@ -1132,9 +1119,8 @@ sub_main(int *exit_code, int argc, const
         case 'U':
           SVN_ERR(svn_utf_cstring_to_utf8(&root_url, arg, pool));
           if (! svn_path_is_url(root_url))
-            handle_error(svn_error_createf(SVN_ERR_INCORRECT_PARAMS, NULL,
-                                           "'%s' is not a URL\n", root_url),
-                         pool);
+            return svn_error_createf(SVN_ERR_INCORRECT_PARAMS, NULL,
+                                     "'%s' is not a URL\n", root_url);
           root_url = sanitize_url(root_url, pool);
           break;
         case 'r':
@@ -1147,11 +1133,9 @@ sub_main(int *exit_code, int argc, const
             if ((! SVN_IS_VALID_REVNUM(base_revision))
                 || (! digits_end)
                 || *digits_end)
-              handle_error(svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR,
-                                             NULL,
-                                             _("Invalid revision number '%s'"),
-                                             saved_arg),
-                           pool);
+              return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+                                       _("Invalid revision number '%s'"),
+                                       saved_arg);
           }
           break;
         case with_revprop_opt:
@@ -1182,12 +1166,11 @@ sub_main(int *exit_code, int argc, const
           break;
         case version_opt:
           SVN_ERR(display_version(opts, pool));
-          exit(EXIT_SUCCESS);
-          break;
+          return SVN_NO_ERROR;
         case 'h':
         case '?':
-          usage(pool, EXIT_SUCCESS);
-          break;
+          usage(stdout, pool);
+          return SVN_NO_ERROR;
         }
     }
 
@@ -1263,13 +1246,16 @@ sub_main(int *exit_code, int argc, const
         action->action = ACTION_PROPDEL;
       else if (! strcmp(action_string, "?") || ! strcmp(action_string, "h")
                || ! strcmp(action_string, "help"))
-        usage(pool, EXIT_SUCCESS);
+        {
+          usage(stdout, pool);
+          return SVN_NO_ERROR;
+        }
       else
-        handle_error(svn_error_createf(SVN_ERR_INCORRECT_PARAMS, NULL,
-                                       "'%s' is not an action\n",
-                                       action_string), pool);
+        return svn_error_createf(SVN_ERR_INCORRECT_PARAMS, NULL,
+                                 "'%s' is not an action\n",
+                                 action_string);
       if (++i == action_args->nelts)
-        insufficient(pool);
+        return insufficient();
 
       /* For copies, there should be a revision number next. */
       if (action->action == ACTION_CP)
@@ -1288,12 +1274,12 @@ sub_main(int *exit_code, int argc, const
 
               action->rev = strtol(rev_str, &end, 0);
               if (*end)
-                handle_error(svn_error_createf(SVN_ERR_INCORRECT_PARAMS, NULL,
-                                               "'%s' is not a revision\n",
-                                               rev_str), pool);
+                return svn_error_createf(SVN_ERR_INCORRECT_PARAMS, NULL,
+                                         "'%s' is not a revision\n",
+                                         rev_str);
             }
           if (++i == action_args->nelts)
-            insufficient(pool);
+            return insufficient();
         }
       else
         {
@@ -1307,7 +1293,7 @@ sub_main(int *exit_code, int argc, const
             svn_dirent_internal_style(APR_ARRAY_IDX(action_args, i,
                                                     const char *), pool);
           if (++i == action_args->nelts)
-            insufficient(pool);
+            return insufficient();
         }
 
       /* For propset, propsetf, and propdel, a property name (and
@@ -1318,7 +1304,7 @@ sub_main(int *exit_code, int argc, const
         {
           action->prop_name = APR_ARRAY_IDX(action_args, i, const char *);
           if (++i == action_args->nelts)
-            insufficient(pool);
+            return insufficient();
 
           if (action->action == ACTION_PROPDEL)
             {
@@ -1330,7 +1316,7 @@ sub_main(int *exit_code, int argc, const
                 svn_string_create(APR_ARRAY_IDX(action_args, i,
                                                 const char *), pool);
               if (++i == action_args->nelts)
-                insufficient(pool);
+                return insufficient();
             }
           else
             {
@@ -1339,7 +1325,7 @@ sub_main(int *exit_code, int argc, const
                                                         const char *), pool);
 
               if (++i == action_args->nelts)
-                insufficient(pool);
+                return insufficient();
 
               SVN_ERR(read_propvalue_file(&(action->prop_value),
                                           propval_file, pool));
@@ -1382,10 +1368,10 @@ sub_main(int *exit_code, int argc, const
           if (! svn_path_is_url(url))
             {
               if (! root_url)
-                handle_error(svn_error_createf(SVN_ERR_INCORRECT_PARAMS, NULL,
-                                               "'%s' is not a URL, and "
-                                               "--root-url (-U) not provided\n",
-                                               url), pool);
+                return svn_error_createf(SVN_ERR_INCORRECT_PARAMS, NULL,
+                                         "'%s' is not a URL, and "
+                                         "--root-url (-U) not provided\n",
+                                         url);
               /* ### These relpaths are already URI-encoded. */
               url = apr_pstrcat(pool, root_url, "/",
                                 svn_relpath_canonicalize(url, pool),
@@ -1407,20 +1393,23 @@ sub_main(int *exit_code, int argc, const
             {
               anchor = svn_uri_get_longest_ancestor(anchor, url, pool);
               if (!anchor || !anchor[0])
-                handle_error(svn_error_createf(SVN_ERR_INCORRECT_PARAMS, NULL,
-                                               "URLs in the action list do not "
-                                               "share a common ancestor"),
-                             pool);
+                return svn_error_createf(SVN_ERR_INCORRECT_PARAMS, NULL,
+                                         "URLs in the action list do not "
+                                         "share a common ancestor");
             }
 
           if ((++i == action_args->nelts) && (j + 1 < num_url_args))
-            insufficient(pool);
+            return insufficient();
         }
       APR_ARRAY_PUSH(actions, struct action *) = action;
     }
 
   if (! actions->nelts)
-    usage(pool, EXIT_FAILURE);
+    {
+      *exit_code = EXIT_FAILURE;
+      usage(stderr, pool);
+      return SVN_NO_ERROR;
+    }
 
   if ((err = execute(actions, anchor, revprops, username, password,
                      config_dir, config_options, non_interactive,
@@ -1431,7 +1420,7 @@ sub_main(int *exit_code, int argc, const
                                    _("Authentication failed and interactive"
                                      " prompting is disabled; see the"
                                      " --force-interactive option"));
-      handle_error(err, pool);
+      return err;
     }
 
   return SVN_NO_ERROR;

Modified: subversion/trunk/subversion/svnserve/svnserve.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnserve/svnserve.c?rev=1544162&r1=1544161&r2=1544162&view=diff
==============================================================================
--- subversion/trunk/subversion/svnserve/svnserve.c (original)
+++ subversion/trunk/subversion/svnserve/svnserve.c Thu Nov 21 12:50:49 2013
@@ -361,7 +361,6 @@ static void usage(const char *progname, 
   svn_error_clear(svn_cmdline_fprintf(stderr, pool,
                                       _("Type '%s --help' for usage.\n"),
                                       progname));
-  exit(1);
 }
 
 static void help(apr_pool_t *pool)
@@ -394,7 +393,6 @@ static void help(apr_pool_t *pool)
       svn_error_clear(svn_cmdline_fprintf(stdout, pool, "  %s\n", optstr));
     }
   svn_error_clear(svn_cmdline_fprintf(stdout, pool, "\n"));
-  exit(0);
 }
 
 static svn_error_t * version(svn_boolean_t quiet, apr_pool_t *pool)
@@ -680,7 +678,11 @@ sub_main(int *exit_code, int argc, const
       if (APR_STATUS_IS_EOF(status))
         break;
       if (status != APR_SUCCESS)
-        usage(argv[0], pool);
+        {
+          usage(argv[0], pool);
+          *exit_code = EXIT_FAILURE;
+          return SVN_NO_ERROR;
+        }
       switch (opt)
         {
         case '6':
@@ -692,7 +694,7 @@ sub_main(int *exit_code, int argc, const
 
         case 'h':
           help(pool);
-          break;
+          return SVN_NO_ERROR;
 
         case 'q':
           quiet = TRUE;
@@ -871,11 +873,15 @@ sub_main(int *exit_code, int argc, const
   if (is_version)
     {
       SVN_ERR(version(quiet, pool));
-      exit(0);
+      return SVN_NO_ERROR;
     }
 
   if (os->ind != argc)
-    usage(argv[0], pool);
+    {
+      usage(argv[0], pool);
+      *exit_code = EXIT_FAILURE;
+      return SVN_NO_ERROR;
+    }
 
   if (mode_opt_count != 1)
     {
@@ -888,6 +894,8 @@ sub_main(int *exit_code, int argc, const
 #endif
                        stderr, pool));
       usage(argv[0], pool);
+      *exit_code = EXIT_FAILURE;
+      return SVN_NO_ERROR;
     }
 
   if (handling_opt_count > 1)
@@ -896,6 +904,8 @@ sub_main(int *exit_code, int argc, const
                       _("You may only specify one of -T or --single-thread\n"),
                       stderr, pool));
       usage(argv[0], pool);
+      *exit_code = EXIT_FAILURE;
+      return SVN_NO_ERROR;
     }
 
   /* construct object pools */
@@ -944,7 +954,8 @@ sub_main(int *exit_code, int argc, const
         (svn_cmdline_fprintf
            (stderr, pool,
             _("Option --tunnel-user is only valid in tunnel mode.\n")));
-      exit(1);
+      *exit_code = EXIT_FAILURE;
+      return SVN_NO_ERROR;
     }
 
   if (run_mode == run_mode_inetd || run_mode == run_mode_tunnel)
@@ -979,7 +990,7 @@ sub_main(int *exit_code, int argc, const
       svn_error_clear(serve(conn, &params, connection_pool));
       svn_pool_destroy(connection_pool);
 
-      exit(0);
+      return SVN_NO_ERROR;
     }
 
 #ifdef WIN32
@@ -1024,7 +1035,8 @@ sub_main(int *exit_code, int argc, const
             }
 
           svn_error_clear(err);
-          exit(1);
+          *exit_code = EXIT_FAILURE;
+          return SVN_NO_ERROR;
         }
 
       /* The service is now in the "starting" state.  Before the SCM will
@@ -1233,7 +1245,7 @@ sub_main(int *exit_code, int argc, const
 
           apr_socket_close(usock);
           apr_socket_close(sock);
-          exit(0);
+          return SVN_NO_ERROR;
         }
 
       switch (handling_mode)
@@ -1246,7 +1258,7 @@ sub_main(int *exit_code, int argc, const
               apr_socket_close(sock);
               svn_error_clear(serve_socket(usock, &params, socket_pool));
               apr_socket_close(usock);
-              exit(0);
+              return SVN_NO_ERROR;
             }
           else if (status == APR_INPARENT)
             {
@@ -1281,28 +1293,19 @@ sub_main(int *exit_code, int argc, const
           status = apr_threadattr_create(&tattr, socket_pool);
           if (status)
             {
-              err = svn_error_wrap_apr(status, _("Can't create threadattr"));
-              svn_handle_error2(err, stderr, FALSE, "svnserve: ");
-              svn_error_clear(err);
-              exit(1);
+              return svn_error_wrap_apr(status, _("Can't create threadattr"));
             }
           status = apr_threadattr_detach_set(tattr, 1);
           if (status)
             {
-              err = svn_error_wrap_apr(status, _("Can't set detached state"));
-              svn_handle_error2(err, stderr, FALSE, "svnserve: ");
-              svn_error_clear(err);
-              exit(1);
+              return svn_error_wrap_apr(status, _("Can't set detached state"));
             }
           status = apr_thread_create(&tid, tattr, serve_thread, thread_data,
                                      shared_pool->pool);
 #endif
           if (status)
             {
-              err = svn_error_wrap_apr(status, THREAD_ERROR_MSG);
-              svn_handle_error2(err, stderr, FALSE, "svnserve: ");
-              svn_error_clear(err);
-              exit(1);
+              return svn_error_wrap_apr(status, THREAD_ERROR_MSG);
             }
           release_shared_pool(shared_pool);
 #endif

Modified: subversion/trunk/subversion/svnversion/svnversion.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnversion/svnversion.c?rev=1544162&r1=1544161&r2=1544162&view=diff
==============================================================================
--- subversion/trunk/subversion/svnversion/svnversion.c (original)
+++ subversion/trunk/subversion/svnversion/svnversion.c Thu Nov 21 12:50:49 2013
@@ -47,7 +47,6 @@ usage(apr_pool_t *pool)
 {
   svn_error_clear(svn_cmdline_fprintf
                   (stderr, pool, _("Type 'svnversion --help' for usage.\n")));
-  exit(1);
 }
 
 
@@ -97,7 +96,6 @@ help(const apr_getopt_option_t *options,
       ++options;
     }
   svn_error_clear(svn_cmdline_fprintf(stdout, pool, "\n"));
-  exit(0);
 }
 
 
@@ -171,7 +169,11 @@ sub_main(int *exit_code, int argc, const
       if (APR_STATUS_IS_EOF(status))
         break;
       if (status != APR_SUCCESS)
-        usage(pool);  /* this will exit() */
+        {
+          *exit_code = EXIT_FAILURE;
+          usage(pool);
+          return SVN_NO_ERROR;
+        }
 
       switch (opt)
         {
@@ -186,22 +188,28 @@ sub_main(int *exit_code, int argc, const
           break;
         case 'h':
           help(options, pool);
-          break;
+          return SVN_NO_ERROR;
         case SVNVERSION_OPT_VERSION:
           is_version = TRUE;
           break;
         default:
-          usage(pool);  /* this will exit() */
+          *exit_code = EXIT_FAILURE;
+          usage(pool);
+          return SVN_NO_ERROR;
         }
     }
 
   if (is_version)
     {
       SVN_ERR(version(quiet, pool));
-      exit(0);
+      return SVN_NO_ERROR;
     }
   if (os->ind > argc || os->ind < argc - 2)
-    usage(pool);  /* this will exit() */
+    {
+      *exit_code = EXIT_FAILURE;
+      usage(pool);
+      return SVN_NO_ERROR;
+    }
 
   SVN_ERR(svn_utf_cstring_to_utf8(&wc_path,
                                   (os->ind < argc) ? os->argv[os->ind] : ".",