Prompted by Julian's review of this freshly-minted public API on IRC:
1. Question to API consumers (eg, client developers): would you find
this API useful? For reference, the current docstring is attached.
2. Should we print the symbolic name in release builds? For example:
% $svn upgrade /
svn: E155019 (WC_INVALID_OP_ON_CWD): Can't upgrade '/' as it is not a
working copy root
svn: E000002: Working copy database '/.svn/wc.db' not found
svn: E000002: Additional errors:
svn: E000002: Can't open file '/.svn/entries': No such file or directory
zsh: exit 1 $svn upgrade /
### (in maintainer mode, this would show
### subversion/libsvn_wc/wc_db.c:15006:
(apr_err=SVN_ERR_WC_INVALID_OP_ON_CWD)
### as well.)
3. Should the API return the error code, or the error code without the
"SVN_ERR_" prefix? I think stripping the error code is better because
then users who search for the error code will be more likely to reach
the users@ archive (with helpful posts) than commits@ archive (which
are unlikely to contain the information they seek). Julian disagrees,
but I'll let him present his own case :-)
I also attached a patch which implements (2), in case we want it.
(It'll be trivial to modify it to suit whichever route we pick for (2)
and (3).)
/**
* Return the symbolic name of an error code. If the error code
* is in svn_error_codes.h, return the name of the macro as a string.
* If the error number is not recognised, return @c NULL.
*
* An error number may not be recognised because it was defined in a future
* version of Subversion (e.g., a 1.9.x server may transmit a defined-in-1.9.0
* error number to a 1.8.x client).
*
* An error number may be recognised @em incorrectly if the @c apr_status_t
* value originates in another library (such as libserf) which also uses APR.
* (This is a theoretical concern only: the @c apr_err member of #svn_error_t
* should never contain a "foreign" @c apr_status_t value, and
* in any case Subversion and Serf use non-overlapping subsets of the
* @c APR_OS_START_USERERR range.)
*
* Support for error codes returned by APR itself (i.e., not in the
* @c APR_OS_START_USERERR range, as defined in apr_errno.h) may be implemented
* in the future.
*
* @note In rare cases, a single numeric code has more than one symbolic name.
* (For example, #SVN_ERR_WC_NOT_DIRECTORY and #SVN_ERR_WC_NOT_WORKING_COPY).
* In those cases, it is not guaranteed which symbolic name is returned.
*
* @since New in 1.8.
*/
const char *
svn_error_symbolic_name(apr_status_t statcode);
Index: subversion/libsvn_subr/error.c
===================================================================
--- subversion/libsvn_subr/error.c (revision 1467305)
+++ subversion/libsvn_subr/error.c (working copy)
@@ -483,9 +483,22 @@ print_error(svn_error_t *err, FILE *stream, const
{
const char *symbolic_name = svn_error_symbolic_name(err->apr_err);
+ const char *start;
+
+ /* ### Inside knowledge: re-add the SVN_ERR_ prefix.
+ (We could iterate error_table, but it's unlikely we'll add more error
+ codes that don't start with "SVN_ERR_" beyond these two.) */
+ if (symbolic_name
+ && !strcmp(symbolic_name, "SVN_WARNING")
+ && !strcmp(symbolic_name, "SVN_NO_ERROR"))
+ start = "";
+ else
+ start = "SVN_ERR_";
+
if (symbolic_name)
svn_error_clear(svn_cmdline_fprintf(stream, err->pool,
- ": (apr_err=%s)\n", symbolic_name));
+ ": (apr_err=%s%s)\n",
+ start, symbolic_name));
else
svn_error_clear(svn_cmdline_fprintf(stream, err->pool,
": (apr_err=%d)\n", err->apr_err));
@@ -500,12 +513,19 @@ print_error(svn_error_t *err, FILE *stream, const
/* Only print the same APR error string once. */
else if (err->message)
{
+ const char *symbolic_name = svn_error_symbolic_name(err->apr_err);
svn_error_clear(svn_cmdline_fprintf(stream, err->pool,
- "%sE%06d: %s\n",
- prefix, err->apr_err, err->message));
+ "%sE%06d%s%s%s: %s\n",
+ prefix, err->apr_err,
+ symbolic_name ? " (" : "",
+ symbolic_name ? symbolic_name : "",
+ symbolic_name ? ")" : "",
+ err->message));
}
else
{
+ const char *symbolic_name = svn_error_symbolic_name(err->apr_err);
+
/* Is this a Subversion-specific error code? */
if ((err->apr_err > APR_OS_START_USEERR)
&& (err->apr_err <= APR_OS_START_CANONERR))
@@ -520,8 +540,12 @@ print_error(svn_error_t *err, FILE *stream, const
}
svn_error_clear(svn_cmdline_fprintf(stream, err->pool,
- "%sE%06d: %s\n",
- prefix, err->apr_err, err_string));
+ "%sE%06d%s%s%s: %s\n",
+ prefix, err->apr_err,
+ symbolic_name ? " (" : "",
+ symbolic_name ? symbolic_name : "",
+ symbolic_name ? ")" : "",
+ err->message));
}
}
@@ -670,7 +694,12 @@ svn_error_symbolic_name(apr_status_t statcode)
for (defn = error_table; defn->errdesc != NULL; ++defn)
if (defn->errcode == (svn_errno_t)statcode)
- return defn->errname;
+ {
+ if (!strncmp(defn->errname, "SVN_ERR_", 8))
+ return defn->errname + 8;
+ else
+ return defn->errname;
+ }
/* "No error" is not in error_table. */
if (statcode == SVN_NO_ERROR)
