You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by gs...@apache.org on 2012/02/08 03:37:34 UTC
svn commit: r1241748 - in /subversion/trunk/subversion: include/svn_editor.h
libsvn_delta/editor.c
Author: gstein
Date: Wed Feb 8 02:37:34 2012
New Revision: 1241748
URL: http://svn.apache.org/viewvc?rev=1241748&view=rev
Log:
Strengthen the discussion and checks around passing all children into
an add_directory() call.
* subversion/include/svn_editor.h:
(...): clarify that children must be mentioned in the parent's
add_directory call.
* subversion/libsvn_delta/editor.c:
(...): reorder some of the marker declarations and add docs
(marker_added_dir, MARKER_ADDED_DIR): new marker
(MARK_RELPATH): helper macro for marking stuff in completed_nodes
(MARK_COMPLETED, MARK_ALLOW_ADD, MARK_ALLOW_ALTER): use MARK_RELPATH
(MARK_ADDED_DIR): new marking helper for nodes from add_directory.
another variant for when checking is disabled.
(CHECK_UNKNOWN_CHILD): new validation to ensure a child under an
added directory was mentioned as a child. plus a variant for when
checking is disabled.
(check_unknown_child): helper for above
(svn_editor_add_directory): mark this specifically as an added dir.
use the new CHECK_UNKNOWN_CHILD() checking macro.
(svn_editor_add_file, svn_editor_add_symlink,
svn_editor_add_absent): use the new CHECK_UNKNOWN_CHILD macro.
Modified:
subversion/trunk/subversion/include/svn_editor.h
subversion/trunk/subversion/libsvn_delta/editor.c
Modified: subversion/trunk/subversion/include/svn_editor.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_editor.h?rev=1241748&r1=1241747&r2=1241748&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_editor.h (original)
+++ subversion/trunk/subversion/include/svn_editor.h Wed Feb 8 02:37:34 2012
@@ -241,10 +241,11 @@ extern "C" {
* follow for each child mentioned in the @a children argument of any
* svn_editor_add_directory() call.
*
- * - ### add section describing: add_* cannot be called for a child of
- * ### a directory added via add_directory in this edit, where that
- * ### child was not mentioned. ie. it should have been passed to the
- * ### parent add_directory so it could be marked as incomplete.
+ * - For each node created with add_*, if its parent was created using
+ * svn_editor_add_directory(), then the new child node MUST have been
+ * mentioned in the @a children parameter of the parent's creation.
+ * This allows the parent directory to properly mark the child as
+ * "incomplete" until the child's add_* call arrives.
*
* - A path should
* never be referenced more than once by the add_*, alter_*, and
Modified: subversion/trunk/subversion/libsvn_delta/editor.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_delta/editor.c?rev=1241748&r1=1241747&r2=1241748&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_delta/editor.c (original)
+++ subversion/trunk/subversion/libsvn_delta/editor.c Wed Feb 8 02:37:34 2012
@@ -65,13 +65,26 @@ struct svn_editor_t
#ifdef ENABLE_ORDERING_CHECK
+/* Marker to indicate no further changes are allowed on this node. */
static const int marker_done;
-static const int marker_allow_add;
-static const int marker_allow_alter;
#define MARKER_DONE (&marker_done)
+
+/* Marker indicating that add_* may be called for this path, or that it
+ can be the destination of a copy or move. For copy/move, the path
+ will switch to MARKER_ALLOW_ALTER, to enable further tweaks. */
+static const int marker_allow_add;
#define MARKER_ALLOW_ADD (&marker_allow_add)
+
+/* Marker indicating that alter_* may be called for this path. */
+static const int marker_allow_alter;
#define MARKER_ALLOW_ALTER (&marker_allow_alter)
+/* Just like MARKER_DONE, but also indicates that the node was created
+ via add_directory(). This allows us to verify that the CHILDREN param
+ was comprehensive. */
+static const int marker_added_dir;
+#define MARKER_ADDED_DIR (&marker_added_dir)
+
#define MARK_FINISHED(editor) ((editor)->finished = TRUE)
#define SHOULD_NOT_BE_FINISHED(editor) SVN_ERR_ASSERT(!(editor)->finished)
@@ -79,28 +92,32 @@ static const int marker_allow_alter;
apr_hash_set((editor)->pending_incomplete_children, relpath, \
APR_HASH_KEY_STRING, NULL);
-#define MARK_COMPLETED(editor, relpath) \
+#define MARK_RELPATH(editor, relpath, value) \
apr_hash_set((editor)->completed_nodes, \
apr_pstrdup((editor)->result_pool, relpath), \
- APR_HASH_KEY_STRING, MARKER_DONE)
+ APR_HASH_KEY_STRING, value)
+
+#define MARK_COMPLETED(editor, relpath) \
+ MARK_RELPATH(editor, relpath, MARKER_DONE)
#define SHOULD_NOT_BE_COMPLETED(editor, relpath) \
SVN_ERR_ASSERT(apr_hash_get((editor)->completed_nodes, relpath, \
APR_HASH_KEY_STRING) == NULL)
#define MARK_ALLOW_ADD(editor, relpath) \
- apr_hash_set((editor)->completed_nodes, \
- apr_pstrdup((editor)->result_pool, relpath), \
- APR_HASH_KEY_STRING, MARKER_ALLOW_ADD)
+ MARK_RELPATH(editor, relpath, MARKER_ALLOW_ADD)
#define SHOULD_ALLOW_ADD(editor, relpath) \
SVN_ERR_ASSERT(allow_either(editor, relpath, MARKER_ALLOW_ADD, NULL))
#define MARK_ALLOW_ALTER(editor, relpath) \
- apr_hash_set((editor)->completed_nodes, \
- apr_pstrdup((editor)->result_pool, relpath), \
- APR_HASH_KEY_STRING, MARKER_ALLOW_ALTER)
+ MARK_RELPATH(editor, relpath, MARKER_ALLOW_ALTER)
#define SHOULD_ALLOW_ALTER(editor, relpath) \
SVN_ERR_ASSERT(allow_either(editor, relpath, MARKER_ALLOW_ALTER, NULL))
+#define MARK_ADDED_DIR(editor, relpath) \
+ MARK_RELPATH(editor, relpath, MARKER_ADDED_DIR)
+#define CHECK_UNKNOWN_CHILD(editor, relpath) \
+ SVN_ERR_ASSERT(check_unknown_child(editor, relpath))
+
static svn_boolean_t
allow_either(const svn_editor_t *editor,
const char *relpath,
@@ -112,6 +129,32 @@ allow_either(const svn_editor_t *editor,
return value == marker1 || value == marker2;
}
+static svn_boolean_t
+check_unknown_child(const svn_editor_t *editor,
+ const char *relpath)
+{
+ const char *parent;
+
+ /* If we already know about the new child, then exit early. */
+ if (apr_hash_get(editor->pending_incomplete_children, relpath,
+ APR_HASH_KEY_STRING) != NULL)
+ return TRUE;
+
+ parent = svn_relpath_dirname(relpath, editor->scratch_pool);
+
+ /* Was this parent created via svn_editor_add_directory() ? */
+ if (apr_hash_get(editor->completed_nodes, relpath, APR_HASH_KEY_STRING)
+ == MARKER_ADDED_DIR)
+ {
+ /* Whoops. This child should have been listed in that add call,
+ and placed into ->pending_incomplete_children. */
+ return FALSE;
+ }
+
+ /* The parent was not added in this drive. */
+ return TRUE;
+}
+
#else
/* Be wary with the definition of these macros so that we don't
@@ -132,6 +175,9 @@ allow_either(const svn_editor_t *editor,
#define MARK_ALLOW_ALTER(editor, relpath) /* empty */
#define SHOULD_ALLOW_ALTER(editor, relpath) /* empty */
+#define MARK_ADDED_DIR(editor, relpath) /* empty */
+#define CHECK_UNKNOWN_CHILD(editor, relpath) /* empty */
+
#endif /* ENABLE_ORDERING_CHECK */
@@ -331,6 +377,7 @@ svn_editor_add_directory(svn_editor_t *e
SVN_ERR_ASSERT(props != NULL);
SHOULD_NOT_BE_FINISHED(editor);
SHOULD_ALLOW_ADD(editor, relpath);
+ CHECK_UNKNOWN_CHILD(editor, relpath);
if (editor->cancel_func)
SVN_ERR(editor->cancel_func(editor->cancel_baton));
@@ -340,7 +387,7 @@ svn_editor_add_directory(svn_editor_t *e
props, replaces_rev,
editor->scratch_pool);
- MARK_COMPLETED(editor, relpath);
+ MARK_ADDED_DIR(editor, relpath);
CLEAR_INCOMPLETE(editor, relpath);
#ifdef ENABLE_ORDERING_CHECK
@@ -378,6 +425,7 @@ svn_editor_add_file(svn_editor_t *editor
SVN_ERR_ASSERT(props != NULL);
SHOULD_NOT_BE_FINISHED(editor);
SHOULD_ALLOW_ADD(editor, relpath);
+ CHECK_UNKNOWN_CHILD(editor, relpath);
if (editor->cancel_func)
SVN_ERR(editor->cancel_func(editor->cancel_baton));
@@ -407,6 +455,7 @@ svn_editor_add_symlink(svn_editor_t *edi
SVN_ERR_ASSERT(props != NULL);
SHOULD_NOT_BE_FINISHED(editor);
SHOULD_ALLOW_ADD(editor, relpath);
+ CHECK_UNKNOWN_CHILD(editor, relpath);
if (editor->cancel_func)
SVN_ERR(editor->cancel_func(editor->cancel_baton));
@@ -433,6 +482,7 @@ svn_editor_add_absent(svn_editor_t *edit
SHOULD_NOT_BE_FINISHED(editor);
SHOULD_ALLOW_ADD(editor, relpath);
+ CHECK_UNKNOWN_CHILD(editor, relpath);
if (editor->cancel_func)
SVN_ERR(editor->cancel_func(editor->cancel_baton));