You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2015/09/14 17:43:31 UTC
svn commit: r1702974 - in /subversion/trunk/subversion: libsvn_subr/token.c
libsvn_wc/wc_db.c
Author: rhuijben
Date: Mon Sep 14 15:43:31 2015
New Revision: 1702974
URL: http://svn.apache.org/r1702974
Log:
Replace an ugly assertion caused by a corrupt working copy database with an
almost as ugly, but better to diagnose error message.
Somehow these few cases which should be completely impossible to trigger
without editing wc.db are reported as TortoiseSVN exception reports.
I explicitly only add these extra checks only to the most common read
operation, as this function is usually called first and catching every
invalid working copy database is impossible without a performance and
a maintenance impact.
* subversion/libsvn_subr/token.c
(svn_token__from_word_err): Ensure NULL is handled safely.
* subversion/libsvn_wc/wc_db.c
(column_token_err): New helper function.
(read_info): Use column_token_err when parsing tokens.
Modified:
subversion/trunk/subversion/libsvn_subr/token.c
subversion/trunk/subversion/libsvn_wc/wc_db.c
Modified: subversion/trunk/subversion/libsvn_subr/token.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/token.c?rev=1702974&r1=1702973&r2=1702974&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/token.c (original)
+++ subversion/trunk/subversion/libsvn_subr/token.c Mon Sep 14 15:43:31 2015
@@ -64,7 +64,7 @@ svn_token__from_word_err(int *value,
if (*value == SVN_TOKEN_UNKNOWN)
return svn_error_createf(SVN_ERR_BAD_TOKEN, NULL,
_("Token '%s' is unrecognized"),
- word);
+ word ? word : "(null)");
return SVN_NO_ERROR;
}
Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1702974&r1=1702973&r2=1702974&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.c Mon Sep 14 15:43:31 2015
@@ -8764,6 +8764,45 @@ svn_wc__db_op_delete_many(svn_wc__db_t *
scratch_pool));
}
+/* Helper function for read_info() to provide better diagnostics than just
+ asserting.
+
+ ### BH: Yes this code is ugly, and that is why I only introduce it in
+ ### read_info(). But we really need something to determine the root cause
+ ### of this problem to diagnose why TortoiseSVN users were seeing all those
+ ### assertions.
+
+ Adds an error to the *err chain if invalid values are encountered. In that
+ case the value is set to the first value in the map, assuming that caller
+ will just return the combined error.
+ */
+static int
+column_token_err(svn_error_t **err,
+ svn_sqlite__stmt_t *stmt,
+ int column,
+ const svn_token_map_t *map)
+{
+ svn_error_t *err2;
+ const char *word = svn_sqlite__column_text(stmt, column, NULL);
+ int value;
+
+ /* svn_token__from_word_err() handles NULL for us */
+ err2 = svn_token__from_word_err(&value, map, word);
+
+ if (err2)
+ {
+ *err = svn_error_compose_create(
+ *err,
+ svn_error_createf(
+ SVN_ERR_WC_CORRUPT, err2,
+ _("Encountered invalid node state in column %d of "
+ "info query to working copy database"),
+ column));
+ value = map[0].val;
+ }
+
+ return value;
+}
/* Like svn_wc__db_read_info(), but taking WCROOT+LOCAL_RELPATH instead of
DB+LOCAL_ABSPATH, and outputting repos ids instead of URL+UUID. */
@@ -8831,11 +8870,11 @@ read_info(svn_wc__db_status_t *status,
svn_node_kind_t node_kind;
op_depth = svn_sqlite__column_int(stmt_info, 0);
- node_kind = svn_sqlite__column_token(stmt_info, 4, kind_map);
+ node_kind = column_token_err(&err, stmt_info, 4, kind_map);
if (status)
{
- *status = svn_sqlite__column_token(stmt_info, 3, presence_map);
+ *status = column_token_err(&err, stmt_info, 3, presence_map);
if (op_depth != 0) /* WORKING */
err = svn_error_compose_create(err,
@@ -8887,14 +8926,11 @@ read_info(svn_wc__db_status_t *status,
if (depth)
{
if (node_kind != svn_node_dir)
- {
- *depth = svn_depth_unknown;
- }
+ *depth = svn_depth_unknown;
+ else if (svn_sqlite__column_is_null(stmt_info, 11))
+ *depth = svn_depth_unknown;
else
- {
- *depth = svn_sqlite__column_token_null(stmt_info, 11, depth_map,
- svn_depth_unknown);
- }
+ *depth = column_token_err(&err, stmt_info, 11, depth_map);
}
if (checksum)
{
Re: svn commit: r1702974 - in /subversion/trunk/subversion:
libsvn_subr/token.c libsvn_wc/wc_db.c
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 14 September 2015 at 17:43, <rh...@apache.org> wrote:
> Author: rhuijben
> Date: Mon Sep 14 15:43:31 2015
> New Revision: 1702974
>
> URL: http://svn.apache.org/r1702974
> Log:
> Replace an ugly assertion caused by a corrupt working copy database with an
> almost as ugly, but better to diagnose error message.
>
> Somehow these few cases which should be completely impossible to trigger
> without editing wc.db are reported as TortoiseSVN exception reports.
>
> I explicitly only add these extra checks only to the most common read
> operation, as this function is usually called first and catching every
> invalid working copy database is impossible without a performance and
> a maintenance impact.
>
> * subversion/libsvn_subr/token.c
> (svn_token__from_word_err): Ensure NULL is handled safely.
>
>
> Modified: subversion/trunk/subversion/libsvn_subr/token.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/token.c?rev=1702974&r1=1702973&r2=1702974&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_subr/token.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/token.c Mon Sep 14 15:43:31 2015
> @@ -64,7 +64,7 @@ svn_token__from_word_err(int *value,
> if (*value == SVN_TOKEN_UNKNOWN)
> return svn_error_createf(SVN_ERR_BAD_TOKEN, NULL,
> _("Token '%s' is unrecognized"),
> - word);
> + word ? word : "(null)");
APR handles NULL strings and replaces it with (null) from 0.9.x. So
this change is unnecessary. We also rely on APR handling for NULL
strings in other places.
--
Ivan Zhakov