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/03/10 17:38:01 UTC
svn commit: r1665610 - in /subversion/trunk/subversion:
libsvn_ra_serf/commit.c libsvn_ra_svn/editorp.c
Author: rhuijben
Date: Tue Mar 10 16:38:00 2015
New Revision: 1665610
URL: http://svn.apache.org/r1665610
Log:
Verify that all directory and file batons are properly closed in the
close_edit() logic of commit for ra_serf, and in the maintainer mode
of ra_svn. The way commit is implemented on ra_serf, not closing batons
might make your commit invalid as properties are collected and
committed all at once.
The bad examples of this problem in our test code were fixed in r1665532.
* subversion/libsvn_ra_serf/commit.c
(commit_context_t): Add variable.
(dir_context_t): Remove unused variable.
(open_root,
add_directory,
open_directory,
close_directory,
add_file,
open_file,
close_file): Track number of open batons.
(close_edit): Return error if some batons are left open.
* subversion/libsvn_ra_svn/editorp.c
(ra_svn_handle_close_edit):
Return error if some batons are left open in maintainer mode.
Modified:
subversion/trunk/subversion/libsvn_ra_serf/commit.c
subversion/trunk/subversion/libsvn_ra_svn/editorp.c
Modified: subversion/trunk/subversion/libsvn_ra_serf/commit.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/commit.c?rev=1665610&r1=1665609&r2=1665610&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/commit.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/commit.c Tue Mar 10 16:38:00 2015
@@ -71,6 +71,7 @@ typedef struct commit_context_t {
const char *checked_in_url; /* checked-in root to base CHECKOUTs from */
const char *vcc_url; /* vcc url */
+ int open_batons; /* Number of open batons */
} commit_context_t;
#define USING_HTTPV2_COMMIT_SUPPORT(commit_ctx) ((commit_ctx)->txn_url != NULL)
@@ -117,9 +118,6 @@ typedef struct dir_context_t {
HTTP v2, for PROPPATCH in HTTP v2). */
const char *url;
- /* How many pending changes we have left in this directory. */
- unsigned int ref_count;
-
/* Is this directory being added? (Otherwise, just opened.) */
svn_boolean_t added;
@@ -1258,6 +1256,8 @@ open_root(void *edit_baton,
const char *proppatch_target = NULL;
apr_pool_t *scratch_pool = svn_pool_create(dir_pool);
+ commit_ctx->open_batons++;
+
if (SVN_RA_SERF__HAVE_HTTPV2_SUPPORT(commit_ctx->session))
{
post_response_ctx_t *prc;
@@ -1533,6 +1533,8 @@ add_directory(const char *path,
dir->name = svn_relpath_basename(dir->relpath, NULL);
dir->prop_changes = apr_hash_make(dir->pool);
+ dir->commit_ctx->open_batons++;
+
if (USING_HTTPV2_COMMIT_SUPPORT(dir->commit_ctx))
{
dir->url = svn_path_url_add_component2(parent->commit_ctx->txn_root_url,
@@ -1623,6 +1625,8 @@ open_directory(const char *path,
dir->name = svn_relpath_basename(dir->relpath, NULL);
dir->prop_changes = apr_hash_make(dir->pool);
+ dir->commit_ctx->open_batons++;
+
if (USING_HTTPV2_COMMIT_SUPPORT(dir->commit_ctx))
{
dir->url = svn_path_url_add_component2(parent->commit_ctx->txn_root_url,
@@ -1701,6 +1705,8 @@ close_directory(void *dir_baton,
proppatch_ctx, dir->pool));
}
+ dir->commit_ctx->open_batons--;
+
return SVN_NO_ERROR;
}
@@ -1720,8 +1726,6 @@ add_file(const char *path,
new_file = apr_pcalloc(file_pool, sizeof(*new_file));
new_file->pool = file_pool;
- dir->ref_count++;
-
new_file->parent_dir = dir;
new_file->commit_ctx = dir->commit_ctx;
new_file->relpath = apr_pstrdup(new_file->pool, path);
@@ -1732,6 +1736,8 @@ add_file(const char *path,
new_file->copy_revision = copy_revision;
new_file->prop_changes = apr_hash_make(new_file->pool);
+ dir->commit_ctx->open_batons++;
+
/* Ensure that the file doesn't exist by doing a HEAD on the
resource. If we're using HTTP v2, we'll just look into the
transaction root tree for this thing. */
@@ -1842,8 +1848,6 @@ open_file(const char *path,
new_file = apr_pcalloc(file_pool, sizeof(*new_file));
new_file->pool = file_pool;
- parent->ref_count++;
-
new_file->parent_dir = parent;
new_file->commit_ctx = parent->commit_ctx;
new_file->relpath = apr_pstrdup(new_file->pool, path);
@@ -1852,6 +1856,8 @@ open_file(const char *path,
new_file->base_revision = base_revision;
new_file->prop_changes = apr_hash_make(new_file->pool);
+ parent->commit_ctx->open_batons++;
+
if (USING_HTTPV2_COMMIT_SUPPORT(parent->commit_ctx))
{
new_file->url = svn_path_url_add_component2(parent->commit_ctx->txn_root_url,
@@ -2014,6 +2020,8 @@ close_file(void *file_baton,
proppatch, scratch_pool));
}
+ ctx->commit_ctx->open_batons--;
+
return SVN_NO_ERROR;
}
@@ -2027,6 +2035,11 @@ close_edit(void *edit_baton,
const svn_commit_info_t *commit_info;
svn_error_t *err = NULL;
+ if (ctx->open_batons > 0)
+ return svn_error_create(
+ SVN_ERR_FS_INCORRECT_EDITOR_COMPLETION, NULL,
+ _("Closing editor with directories or files open"));
+
/* MERGE our activity */
SVN_ERR(svn_ra_serf__run_merge(&commit_info,
ctx->session,
Modified: subversion/trunk/subversion/libsvn_ra_svn/editorp.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_svn/editorp.c?rev=1665610&r1=1665609&r2=1665610&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_svn/editorp.c (original)
+++ subversion/trunk/subversion/libsvn_ra_svn/editorp.c Tue Mar 10 16:38:00 2015
@@ -843,6 +843,14 @@ static svn_error_t *ra_svn_handle_close_
{
SVN_CMD_ERR(ds->editor->close_edit(ds->edit_baton, pool));
ds->done = TRUE;
+#ifdef SVN_DEBUG
+ /* Before enabling this in non-maintainer mode:
+ * Note that this code is used on both client *and* server */
+ if (apr_hash_count(ds->tokens) != 0)
+ return svn_error_create(
+ SVN_ERR_FS_INCORRECT_EDITOR_COMPLETION, NULL,
+ _("Closing editor with directories or files open"));
+#endif
if (ds->aborted)
*ds->aborted = FALSE;
return svn_ra_svn__write_cmd_response(conn, pool, "");