On 11 February 2015 at 03:42, <rhuijben_at_apache.org> wrote:
> Author: rhuijben
> Date: Wed Feb 11 00:42:01 2015
> New Revision: 1658848
>
> URL: http://svn.apache.org/r1658848
> Log:
> Make the (non recursive) revert db operation properly report tree conflicts
> that it creates by both fixing what is stored in the tree conflict and by
> properly creating notifications.
>
> Note that a recursive revert wouldn't encounter this problem as it
> would just break the moves.
>
Hi Bert,
It seems this commit introduces potential use of uninitialized
variable CONFLICT in subversion/libsvn_wc/wc_db.c:op_revert_txn(). See
below.
> Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1658848&r1=1658847&r2=1658848&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/wc_db.c Wed Feb 11 00:42:01 2015
> @@ -6738,6 +6738,8 @@ op_revert_txn(void *baton,
> svn_boolean_t moved_here;
> int affected_rows;
> const char *moved_to;
> + int op_depth_increased = 0;
> + svn_skel_t *conflict;
CONFLICT local variable will be uninitialized if MOVED_TO is non-zero.
>
> /* ### Similar structure to op_revert_recursive_txn, should they be
> combined? */
> @@ -6794,45 +6796,12 @@ op_revert_txn(void *baton,
> }
> else
> {
> - svn_skel_t *conflict;
> -
> SVN_ERR(svn_wc__db_read_conflict_internal(&conflict, wcroot,
> local_relpath,
> scratch_pool, scratch_pool));
> - if (conflict)
> - {
> - svn_wc_operation_t operation;
> - svn_boolean_t tree_conflicted;
> -
> - SVN_ERR(svn_wc__conflict_read_info(&operation, NULL, NULL, NULL,
> - &tree_conflicted,
> - db, wcroot->abspath,
> - conflict,
> - scratch_pool, scratch_pool));
> - if (tree_conflicted
> - && (operation == svn_wc_operation_update
> - || operation == svn_wc_operation_switch))
> - {
> - svn_wc_conflict_reason_t reason;
> - svn_wc_conflict_action_t action;
> -
> - SVN_ERR(svn_wc__conflict_read_tree_conflict(&reason, &action,
> - NULL,
> - db, wcroot->abspath,
> - conflict,
> - scratch_pool,
> - scratch_pool));
> -
> - if (reason == svn_wc_conflict_reason_deleted)
> - SVN_ERR(svn_wc__db_resolve_delete_raise_moved_away(
> - db, svn_dirent_join(wcroot->abspath, local_relpath,
> - scratch_pool),
> - NULL, NULL /* ### How do we notify this? */,
> - scratch_pool));
> - }
> - }
> }
>
> +
> if (op_depth > 0 && op_depth == relpath_depth(local_relpath))
> {
> /* Can't do non-recursive revert if children exist */
> @@ -6857,7 +6826,7 @@ op_revert_txn(void *baton,
> SVN_ERR(svn_sqlite__bindf(stmt, "isd", wcroot->wc_id,
> local_relpath,
> op_depth));
> - SVN_ERR(svn_sqlite__step_done(stmt));
> + SVN_ERR(svn_sqlite__update(&op_depth_increased, stmt));
>
> SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
> STMT_DELETE_WORKING_NODE));
> @@ -6875,6 +6844,54 @@ op_revert_txn(void *baton,
> SVN_ERR(clear_moved_to(wcroot, local_relpath, scratch_pool));
> }
>
> + if (op_depth_increased && conflict)
> + {
> + svn_wc_operation_t operation;
> + svn_boolean_t tree_conflicted;
> + const apr_array_header_t *locations;
> +
> + SVN_ERR(svn_wc__conflict_read_info(&operation, &locations, NULL, NULL,
> + &tree_conflicted,
> + db, wcroot->abspath,
> + conflict,
> + scratch_pool, scratch_pool));
And then used here.
I'm not expert in this area, but may you know how to reproduce and fix
this problem because I see several TortoiseSVN crashes [1] due access
to uninitialized variable CONFLICT. Call stack:
[[[
libsvn_tsvn.dll!svn_wc__conflict_read_info(svn_wc_operation_t *
operation, const apr_array_header_t * * locations, int *
text_conflicted, int * prop_conflicted, int * tree_conflicted,
svn_wc__db_t * conflict_skel, const char * result_pool, const
svn_skel_t * scratch_pool, apr_pool_t *) Line 700 C
> libsvn_tsvn.dll!op_revert_txn(void * baton, svn_wc__db_wcroot_t * wcroot, const char * local_relpath, apr_pool_t * scratch_pool) Line 6946 C
libsvn_tsvn.dll!with_triggers(void * baton, svn_wc__db_wcroot_t *
wcroot, const char * local_relpath, apr_pool_t * scratch_pool) Line
3081 C
libsvn_tsvn.dll!svn_wc__db_op_revert(svn_wc__db_t * db, const
char * local_abspath, svn_depth_t depth, int clear_changelists,
apr_pool_t * scratch_pool, apr_pool_t *) Line 7209 C
libsvn_tsvn.dll!revert(svn_wc__db_t * db, const char *
local_abspath, svn_depth_t depth, int use_commit_times, int
clear_changelists, int metadata_only, svn_error_t * (void *) *
cancel_func, void * cancel_baton, void (void *, const svn_wc_notify_t
*, apr_pool_t *) * notify_func, void * notify_baton, apr_pool_t *
scratch_pool) Line 743 C
libsvn_tsvn.dll!svn_wc_revert5(svn_wc_context_t * wc_ctx, const
char * local_abspath, svn_depth_t depth, int use_commit_times, const
apr_array_header_t * changelist_filter, int clear_changelists, int
metadata_only, svn_error_t * (void *) * cancel_func, void *
cancel_baton, void (void *, const svn_wc_notify_t *, apr_pool_t *) *
notify_func, void * notify_baton, apr_pool_t * scratch_pool) Line 995
C
]]
[1] https://drdump.com/Bug.aspx?ProblemID=146007 (private)
--
Ivan Zhakov
Received on 2015-09-09 20:00:41 CEST