On May 9, 2009, at 2:16 PM, C. Michael Pilato wrote:
> Author: cmpilato
> Date: Sat May 9 12:16:40 2009
> New Revision: 37662
>
> Log:
> Add a new public API for trimming out the trace-only error chain
> links,
> and use it in mod_dav_svn to avoid sending useless error messages
> across
> the wire. This makes still more tests pass over WebDAV.
>
> * subversion/include/svn_error.h
> (svn_err_purge_tracing): New function prototype.
>
> * subversion/libsvn_subr/error.c
> (is_tracing_link): New function, taking the place of ...
> (is_trace_message): ... this now-removed one. Callers updated to
> call
> is_tracing_link() instead.
> (svn_err_purge_tracing): New function.
>
> * subversion/mod_dav_svn/version.c
> (merge): Use svn_err_purge_tracing() before squirreling away the
> post-commit hook error message.
>
> * subversion/mod_dav_svn/util.c
> (build_error_chain): Revert changes made in r37649.
> (dav_svn__convert_err): Use svn_err_purge_tracing() before operating
> on the Subversion error.
>
> Modified:
> trunk/subversion/include/svn_error.h
> trunk/subversion/libsvn_subr/error.c
> trunk/subversion/mod_dav_svn/util.c
> trunk/subversion/mod_dav_svn/version.c
>
> Modified: trunk/subversion/include/svn_error.h
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/include/svn_error.h?pathrev=37662&r1=37661&r2=37662
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- trunk/subversion/include/svn_error.h Sat May 9 12:06:18 2009
> (r37661)
> +++ trunk/subversion/include/svn_error.h Sat May 9 12:16:40 2009
> (r37662)
> @@ -297,6 +297,18 @@ svn_handle_warning(FILE *stream,
> #define svn_error_return(expr) (expr)
> #endif
>
> +/**
> + * Purge from @a ERR and its child chain any links associated with
> + * error tracing placeholders, and return the new top-level error
> + * chain item. @a ERR should be considered unusable after passing
> + * through this function, but should *not* be cleared (as the
> returned
> + * error is shares memory with @a ERR).
> + *
> + * @since New in 1.6.
> + */
> +svn_error_t *svn_err_purge_tracing(svn_error_t *err);
> +
> +
> /** A statement macro, very similar to @c SVN_ERR.
> *
> * This macro will wrap the error with the specified text before
>
> Modified: trunk/subversion/libsvn_subr/error.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_subr/error.c?pathrev=37662&r1=37661&r2=37662
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- trunk/subversion/libsvn_subr/error.c Sat May 9 12:06:18 2009
> (r37661)
> +++ trunk/subversion/libsvn_subr/error.c Sat May 9 12:16:40 2009
> (r37662)
> @@ -311,16 +311,63 @@ svn_error_clear(svn_error_t *err)
> }
> }
>
> -/* Is MESSAGE the message used for the "fake" errors used in
> tracing? */
> -static svn_boolean_t is_trace_message(const char *message)
> +/* Is ERR a tracing-only error chain link? */
> +static svn_boolean_t is_tracing_link(svn_error_t *err)
> {
> #ifdef SVN_ERR__TRACING
> - return (message != NULL && !strcmp(message, SVN_ERR__TRACED));
> + /* ### A strcmp()? Really? I think it's the best we can do unless
> + ### we add a boolean field to svn_error_t that's set only for
> + ### these "placeholder error chain" items. Not such a bad idea,
> + ### really... */
> + return (err && err->message && !strcmp(err->message,
> SVN_ERR__TRACED));
> #else
> return FALSE;
> #endif
> }
>
> +svn_error_t *
> +svn_err_purge_tracing(svn_error_t *err)
> +{
> +#ifdef SVN_ERR__TRACING
> + svn_error_t *tmp_err = err;
> + svn_error_t *new_err = NULL, *new_err_leaf = NULL;
> +
> + if (! err)
> + return SVN_NO_ERROR;
> +
> + while (tmp_err)
> + {
> + /* Skip over any trace-only links. */
> + while (tmp_err && is_tracing_link(tmp_err))
> + tmp_err = tmp_err->child;
> +
> + /* Add a new link to the new chain (creating the chain if
> necessary). */
> + if (! new_err)
> + {
> + new_err = tmp_err;
> + new_err_leaf = new_err;
> + }
> + else
> + {
> + new_err_leaf->child = tmp_err;
> + new_err_leaf = new_err_leaf->child;
> + }
> +
> + /* Advance to the next link in the original chain. */
> + tmp_err = tmp_err->child;
> + }
> +
> + /* If we get here, there had better be a real link in this error
> chain. */
> + assert(new_err_leaf);
Perhaps use SVN_ERR_ASSERT()?
>
> +
> + /* Tie off the chain, and return its head. */
> + new_err_leaf->child = NULL;
> + return new_err;
> +#else /* SVN_ERR__TRACING */
> + return err;
> +#endif /* SVN_ERR__TRACING */
> +}
> +
> static void
> print_error(svn_error_t *err, FILE *stream, const char *prefix)
> {
> @@ -353,7 +400,7 @@ print_error(svn_error_t *err, FILE *stre
> #endif /* SVN_DEBUG */
>
> /* "traced call" */
> - if (is_trace_message(err->message))
> + if (is_tracing_link(err))
> {
> /* Skip it. We already printed the file-line coordinates. */
> }
> @@ -472,7 +519,7 @@ svn_handle_warning2(FILE *stream, svn_er
> char buf[256];
>
> /* Skip over any trace records. */
> - while (is_trace_message(err->message))
> + while (is_tracing_link(err))
> err = err->child;
>
> svn_error_clear(svn_cmdline_fprintf
>
> Modified: trunk/subversion/mod_dav_svn/util.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/mod_dav_svn/util.c?pathrev=37662&r1=37661&r2=37662
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- trunk/subversion/mod_dav_svn/util.c Sat May 9 12:06:18 2009
> (r37661)
> +++ trunk/subversion/mod_dav_svn/util.c Sat May 9 12:16:40 2009
> (r37662)
> @@ -55,31 +55,14 @@ dav_svn__new_error_tag(apr_pool_t *pool,
> static dav_error *
> build_error_chain(apr_pool_t *pool, svn_error_t *err, int status)
> {
> - svn_error_t *serr = err;
> - dav_error *derr;
> + char *msg = err->message ? apr_pstrdup(pool, err->message) : NULL;
>
> -#ifdef SVN_ERR__TRACING
> - /* If we're generated full error tracing chains, we need to trim
> - the placeholder chain items out of the mix here -- our
> Subversion
> - client wants predictable output from its server.
> -
> - ### A strcmp()? Really? I think it's the best we can do unless
> - ### we add a boolean field to svn_error_t that's set only for
> - ### these "placeholder error chain" items. Not such a bad idea,
> - ### really...
> - */
> - while (serr && serr->message
> - && (strcmp(serr->message, SVN_ERR__TRACED) == 0))
> - serr = serr->child;
> -#endif
> -
> - derr = dav_svn__new_error_tag(pool, status, serr->apr_err,
> - serr->message ? apr_pstrdup(pool,
> - serr-
> >message)
> - : NULL,
> - SVN_DAV_ERROR_NAMESPACE,
> SVN_DAV_ERROR_TAG);
> - if (serr->child)
> - derr->prev = build_error_chain(pool, serr->child, status);
> + dav_error *derr = dav_svn__new_error_tag(pool, status, err-
> >apr_err, msg,
> + SVN_DAV_ERROR_NAMESPACE,
> + SVN_DAV_ERROR_TAG);
> +
> + if (err->child)
> + derr->prev = build_error_chain(pool, err->child, status);
>
> return derr;
> }
> @@ -93,6 +76,11 @@ dav_svn__convert_err(svn_error_t *serr,
> {
> dav_error *derr;
>
> + /* Remove the trace-only error chain links. We need predictable
> + protocol behavior regardless of whether or not we're in a
> + debugging build. */
> + serr = svn_err_purge_tracing(serr);
> +
> /* ### someday mod_dav_svn will send back 'rich' error tags, much
> finer grained than plain old svn_error_t's. But for now, all
> svn_error_t's are marshalled to the client via the single
>
> Modified: trunk/subversion/mod_dav_svn/version.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/mod_dav_svn/version.c?pathrev=37662&r1=37661&r2=37662
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- trunk/subversion/mod_dav_svn/version.c Sat May 9 12:06:18 2009
> (r37661)
> +++ trunk/subversion/mod_dav_svn/version.c Sat May 9 12:16:40 2009
> (r37662)
> @@ -1399,6 +1399,7 @@ merge(dav_resource *target,
> }
> else if (serr)
> {
> + serr = svn_err_purge_tracing(serr);
> if (serr->child && serr->child->message)
> post_commit_err = apr_pstrdup(pool, serr->child->message);
> svn_error_clear(serr);
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageId=2150402
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2151668
Received on 2009-05-09 22:31:35 CEST