hwri...@apache.org writes:
> Author: hwright
> Date: Thu Nov 19 18:09:08 2009
> New Revision: 882232
> --- subversion/trunk/subversion/libsvn_client/revert.c (original)
> +++ subversion/trunk/subversion/libsvn_client/revert.c Thu Nov 19 18:09:08
> 2009
> @@ -68,19 +68,13 @@
> svn_client_ctx_t *ctx,
> apr_pool_t *pool)
> {
> - svn_wc_adm_access_t *adm_access, *target_access;
> - const char *target, *local_abspath;
> + const char *local_abspath;
> svn_error_t *err;
> - int adm_lock_level = SVN_WC__LEVELS_TO_LOCK_FROM_DEPTH(depth);
> -
> - SVN_ERR(svn_wc__adm_open_anchor_in_context(
> - &adm_access, &target_access, &target,
> - ctx->wc_ctx, path, TRUE, adm_lock_level,
> - ctx->cancel_func, ctx->cancel_baton,
> - pool));
>
> SVN_ERR(svn_dirent_get_absolute(&local_abspath, path, pool));
>
> + SVN_ERR(svn_wc__acquire_write_lock(ctx->wc_ctx, local_abspath, pool));
> +
> err = svn_wc_revert4(ctx->wc_ctx,
> local_abspath,
> depth,
> @@ -108,7 +102,8 @@
> return svn_error_return(err);
> }
>
> - return svn_wc_adm_close2(adm_access, pool);
> + return svn_error_return(
> + svn_wc__release_write_lock(ctx->wc_ctx, local_abspath, pool));
> }
Your change conflicts with my change shown below. The problem is
shown by depth_tests 33 which reverts a schedule add directory. Your
code above assumes after calling svn_wc_revert4 on such a directory it
is still possible to call svn_wc__release_write_lock. Is that valid
given that revert makes the whole directory vanish?
Note that in the 1.6 code it was possible to close access batons more
than once, and that r881265 introduced what is in my view an erroneous
skip of a null access baton.
My change:
A better fix for the SEGV fixed in r881265: replace another use
of access batons.
* subversion/libsvn_wc/adm_files.h
(svn_wc__adm_destroy): Pass svn_wc__db_t and abspath rather than
svn_wc_adm_access_t.
* subversion/libsvn_wc/adm_files.c
(svn_wc__adm_destroy): Use svn_wc__db_t directly.
* subversion/libsvn_wc/adm_ops.c
(svn_wc__internal_remove_from_revision_control): Revert r881265: no
need to check for null access baton.
Index: subversion/libsvn_wc/adm_ops.c
===================================================================
--- subversion/libsvn_wc/adm_ops.c (revision 882277)
+++ subversion/libsvn_wc/adm_ops.c (working copy)
@@ -2528,14 +2528,8 @@
/* Remove the entire administrative .svn area, thereby removing
_this_ dir from revision control too. */
- {
- svn_wc_adm_access_t *adm_access =
- svn_wc__adm_retrieve_internal2(db, local_abspath, iterpool);
+ SVN_ERR(svn_wc__adm_destroy(db, local_abspath, iterpool));
- if (adm_access)
- SVN_ERR(svn_wc__adm_destroy(adm_access, iterpool));
- }
-
/* If caller wants us to recursively nuke everything on disk, go
ahead, provided that there are no dangling local-mod files
below */
Index: subversion/libsvn_wc/adm_files.c
===================================================================
--- subversion/libsvn_wc/adm_files.c (revision 882277)
+++ subversion/libsvn_wc/adm_files.c (working copy)
@@ -611,26 +611,21 @@
}
svn_error_t *
-svn_wc__adm_destroy(svn_wc_adm_access_t *adm_access,
+svn_wc__adm_destroy(svn_wc__db_t *db,
+ const char *dir_abspath,
apr_pool_t *scratch_pool)
{
- const char *path, *local_abspath;
- svn_wc__db_t *db = svn_wc__adm_get_db(adm_access);
+ const char *fullpath;
- SVN_ERR(svn_wc__write_check(db, svn_wc__adm_access_abspath(adm_access),
- scratch_pool));
+ SVN_ERR(svn_wc__write_check(db, dir_abspath, scratch_pool));
/* Well, the coast is clear for blowing away the administrative
- directory, which also removes the lock file */
- path = svn_wc__adm_child(svn_wc_adm_access_path(adm_access), NULL,
- scratch_pool);
- local_abspath = svn_wc__adm_access_abspath(adm_access);
+ directory, which also removes the lock */
+ SVN_ERR(svn_wc__db_temp_forget_directory(db, dir_abspath, scratch_pool));
- SVN_ERR(svn_wc_adm_close2(adm_access, scratch_pool));
- SVN_ERR(svn_wc__db_temp_forget_directory(db, local_abspath, scratch_pool));
+ fullpath = extend_with_adm_name(dir_abspath, NULL, FALSE, scratch_pool,
NULL);
+ SVN_ERR(svn_io_remove_dir2(fullpath, FALSE, NULL, NULL, scratch_pool));
- SVN_ERR(svn_io_remove_dir2(path, FALSE, NULL, NULL, scratch_pool));
-
return SVN_NO_ERROR;
}
Index: subversion/libsvn_wc/adm_files.h
===================================================================
--- subversion/libsvn_wc/adm_files.h (revision 882277)
+++ subversion/libsvn_wc/adm_files.h (working copy)
@@ -130,10 +130,9 @@
apr_pool_t *scratch_pool);
-/* Blow away the admistrative directory associated with the access baton
- ADM_ACCESS. This closes ADM_ACCESS, but it is safe to close ADM_ACCESS
- again, after calling this function. */
-svn_error_t *svn_wc__adm_destroy(svn_wc_adm_access_t *adm_access,
+/* Blow away the admistrative directory associated with DIR_ABSPATH */
+svn_error_t *svn_wc__adm_destroy(svn_wc__db_t *db,
+ const char *dir_abspath,
apr_pool_t *scratch_pool);
--
Philip
