Re: [PATCH] a little step in issue 897: non-recursive svn_handle_error()

Erik Hülsmann wrote:

>Hi!
>
>
>In the discussion about issue #897 Greg Hudson suggests that
>svn_handle_error() is too lispy and could/should be rewritten
>to be more C-like.
>
>
I really cannot comprehend this aversion towards recursion. "More
C-like" indeed; as if recursion was a language feature, not an
implementation tool. Bah.

Then there's the issue of changing the whole function into one huge
loop, which makes the it harder to read and understand. It's better to
keep loops as small as is feasible.

Index: error.c
===================================================================
--- error.c (revision 7264)
+++ error.c (working copy)
@@ -184,7 +184,7 @@
 
 static void
 handle_error (svn_error_t *err, FILE *stream, svn_boolean_t fatal,
- int depth, apr_status_t parent_apr_err)
+ svn_boolean_t print_strerror)
 {
   char errbuf[256];
   const char *err_string;
@@ -203,9 +203,8 @@
   fprintf (stream, ": (apr_err=%d)\n", err->apr_err);
 #endif /* SVN_DEBUG */
 
- /* When we're recursing, don't repeat the top-level message if its
- the same as before. */
- if (depth == 0 || err->apr_err != parent_apr_err)
+ /* Only print the same APR error string once. */
+ if (print_strerror)
     {
       /* Is this a Subversion-specific error code? */
       if ((err->apr_err > APR_OS_START_USEERR)
@@ -223,9 +222,6 @@
              convert_string_for_output (err->message, err->pool));
   fflush (stream);
 
- if (err->child)
- handle_error (err->child, stream, FALSE, depth + 1, err->apr_err);
-
   if (fatal)
     /* XXX Shouldn't we exit(1) here instead, so that atexit handlers
        get called? --xbc */
@@ -235,7 +231,13 @@
 void
 svn_handle_error (svn_error_t *err, FILE *stream, svn_boolean_t fatal)
 {
- handle_error (err, stream, fatal, 0, APR_SUCCESS);
+ apr_status_t parent_apr_err = APR_SUCCESS;
+ while (err)
+ {
+ handle_error (err, stream, fatal, (err->apr_err == parent_apr_err));
+ parent_apr_err = err->apr_err;
+ err = err->child;
+ }
 }
 
 

-- 
Brane ÄŒibej   <brane_at_xbc.nu>   http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org