subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From julianf...@apache.org
Subject svn commit: r936810 - /subversion/trunk/subversion/libsvn_client/commit.c
Date Thu, 22 Apr 2010 12:22:48 GMT
Author: julianfoad
Date: Thu Apr 22 12:22:48 2010
New Revision: 936810

URL: http://svn.apache.org/viewvc?rev=936810&view=rev
Log:
Remove some uses of svn_iter_apr_array() and use simple C loops instead,
because that method caused too much obfuscation.  This restores the
iteration to roughly how it was before the new method was introduced in
r866607, but keeping the larger loop bodies factored out into separate
functions while the smaller ones now go back to in-line code.

No functional change.

Note:  As well as introducing and using the iteration functions, r866607
made some changes to pool usage.  One such change was in the code involved
in this commit.  In the loop where svn_client_commit4() calls
post_process_commit_item(), it previously allocated the queue in a sub-pool
(for want of a scratch pool), but subsequently has been allocating it in the
function's main pool, and continues to do so after this commit.  We should
consider whether that was an accidental regression and, if so, restore the
use of a sub-pool (or scratch pool).

* subversion/libsvn_client/commit.c
  (post_commit_baton, check_dir_delete_baton): Delete.
  (post_process_commit_item, check_nonrecursive_dir_delete): Take arguments
    through function parameters rather than through a 'baton' structure.
  (commit_item_is_changed): Delete, moving the body into svn_client_commit4().
  (svn_client_commit4): Iterate over arrays directly rather than through
    svn_iter_apr_array().

Modified:
    subversion/trunk/subversion/libsvn_client/commit.c

Modified: subversion/trunk/subversion/libsvn_client/commit.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/commit.c?rev=936810&r1=936809&r2=936810&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/commit.c (original)
+++ subversion/trunk/subversion/libsvn_client/commit.c Thu Apr 22 12:22:48 2010
@@ -46,7 +46,6 @@
 #include "svn_time.h"
 #include "svn_sorts.h"
 #include "svn_props.h"
-#include "svn_iter.h"
 
 #include "client.h"
 #include "private/svn_wc_private.h"
@@ -934,28 +933,22 @@ collect_lock_tokens(apr_hash_t **result,
   return SVN_NO_ERROR;
 }
 
-struct post_commit_baton
-{
-  svn_wc_committed_queue_t *queue;
-  apr_pool_t *qpool;
-  svn_wc_context_t *wc_ctx;
-  svn_boolean_t keep_changelists;
-  svn_boolean_t keep_locks;
-  apr_hash_t *checksums;
-};
-
+/* Put ITEM onto QUEUE... */
 static svn_error_t *
-post_process_commit_item(void *baton, void *this_item, apr_pool_t *pool)
+post_process_commit_item(svn_wc_committed_queue_t *queue,
+                         svn_client_commit_item3_t *item,
+                         svn_wc_context_t *wc_ctx,
+                         svn_boolean_t keep_changelists,
+                         svn_boolean_t keep_locks,
+                         apr_hash_t *checksums,
+                         apr_pool_t *result_pool,
+                         apr_pool_t *pool)
 {
-  struct post_commit_baton *btn = baton;
-
-  svn_client_commit_item3_t *item =
-    *(svn_client_commit_item3_t **)this_item;
   svn_boolean_t loop_recurse = FALSE;
   svn_boolean_t remove_lock;
 
   /* Is it a missing, deleted directory?
- 
+
      ### Temporary: once we centralise this sort of node is just a
      normal delete and will get handled by the post-commit queue. */
   if (item->kind == svn_node_dir
@@ -964,10 +957,10 @@ post_process_commit_item(void *baton, vo
       svn_boolean_t obstructed;
 
       SVN_ERR(svn_wc__node_is_status_obstructed(&obstructed,
-                                                btn->wc_ctx, item->path, pool));
+                                                wc_ctx, item->path, pool));
       if (obstructed)
         return svn_wc__temp_mark_missing_not_present(item->path,
-                                                     btn->wc_ctx, pool);
+                                                     wc_ctx, pool);
     }
 
   if ((item->state_flags & SVN_CLIENT_COMMIT_ITEM_ADD)
@@ -975,57 +968,41 @@ post_process_commit_item(void *baton, vo
       && (item->copyfrom_url))
     loop_recurse = TRUE;
 
-  remove_lock = (! btn->keep_locks && (item->state_flags
+  remove_lock = (! keep_locks && (item->state_flags
                                        & SVN_CLIENT_COMMIT_ITEM_LOCK_TOKEN));
 
   /* Allocate the queue in a longer-lived pool than (iter)pool:
      we want it to survive the next iteration. */
-  return svn_wc_queue_committed3(btn->queue, item->path,
+  return svn_wc_queue_committed3(queue, item->path,
                                  loop_recurse, item->incoming_prop_changes,
-                                 remove_lock, !btn->keep_changelists,
-                                 apr_hash_get(btn->checksums,
+                                 remove_lock, !keep_changelists,
+                                 apr_hash_get(checksums,
                                               item->path,
                                               APR_HASH_KEY_STRING),
-                                 btn->qpool);
+                                 result_pool);
 }
 
 
 static svn_error_t *
-commit_item_is_changed(void *baton, void *this_item, apr_pool_t *pool)
+check_nonrecursive_dir_delete(const char *target_path,
+                              svn_wc_context_t *wc_ctx,
+                              svn_depth_t depth,
+                              apr_pool_t *pool)
 {
-  svn_client_commit_item3_t **item = this_item;
-
-  if ((*item)->state_flags != SVN_CLIENT_COMMIT_ITEM_LOCK_TOKEN)
-    svn_iter_break(pool);
-
-  return SVN_NO_ERROR;
-}
-
-struct check_dir_delete_baton
-{
-  svn_wc_context_t *wc_ctx;
-  svn_depth_t depth;
-};
-
-static svn_error_t *
-check_nonrecursive_dir_delete(void *baton, void *this_item, apr_pool_t *pool)
-{
-  struct check_dir_delete_baton *btn = baton;
   const char *target_abspath, *lock_abspath;
   svn_boolean_t locked_here;
   svn_node_kind_t kind;
 
-  SVN_ERR(svn_dirent_get_absolute(&target_abspath, *(const char **)this_item,
-                                  pool));
+  SVN_ERR(svn_dirent_get_absolute(&target_abspath, target_path, pool));
 
-  SVN_ERR(svn_wc__node_get_kind(&kind, btn->wc_ctx, target_abspath, FALSE,
+  SVN_ERR(svn_wc__node_get_kind(&kind, wc_ctx, target_abspath, FALSE,
                                 pool));
   if (kind == svn_node_dir)
     lock_abspath = target_abspath;
   else
     lock_abspath = svn_dirent_dirname(target_abspath, pool);
 
-  SVN_ERR(svn_wc_locked2(&locked_here, NULL, btn->wc_ctx, lock_abspath, pool));
+  SVN_ERR(svn_wc_locked2(&locked_here, NULL, wc_ctx, lock_abspath, pool));
   if (!locked_here)
     return svn_error_create(SVN_ERR_WC_LOCKED, NULL,
                            _("Are all targets part of the same working copy?"));
@@ -1046,7 +1023,7 @@ check_nonrecursive_dir_delete(void *bato
      ### This would be fairly easy to fix, though: just, well,
      ### check the above conditions!
   */
-  if (btn->depth != svn_depth_infinity)
+  if (depth != svn_depth_infinity)
     {
       if (kind == svn_node_dir)
         {
@@ -1054,14 +1031,14 @@ check_nonrecursive_dir_delete(void *bato
 
           /* ### Looking at schedule is probably enough, no need for
                  pristine compare etc. */
-          SVN_ERR(svn_wc_status3(&status, btn->wc_ctx, target_abspath, pool,
+          SVN_ERR(svn_wc_status3(&status, wc_ctx, target_abspath, pool,
                                  pool));
           if (status->text_status == svn_wc_status_deleted ||
               status->text_status == svn_wc_status_replaced)
             {
               const apr_array_header_t *children;
 
-              SVN_ERR(svn_wc__node_get_children(&children, btn->wc_ctx,
+              SVN_ERR(svn_wc__node_get_children(&children, wc_ctx,
                                                 target_abspath, TRUE, pool,
                                                 pool));
 
@@ -1157,15 +1134,13 @@ svn_client_commit4(svn_commit_info_t **c
 
      At the same time, if a non-recursive commit is desired, do not
      allow a deleted directory as one of the targets. */
-  {
-    struct check_dir_delete_baton btn;
+  for (i = 0; i < targets->nelts; i++)
+    {
+      const char *target_path = APR_ARRAY_IDX(targets, i, const char *);
 
-    btn.wc_ctx = ctx->wc_ctx;
-    btn.depth = depth;
-    SVN_ERR(svn_iter_apr_array(NULL, targets,
-                               check_nonrecursive_dir_delete, &btn,
-                               pool));
-  }
+      SVN_ERR(check_nonrecursive_dir_delete(target_path, ctx->wc_ctx, depth,
+                                            /*iter*/pool));
+    }
 
   /* Crawl the working copy for commit items. */
   if ((cmt_err = svn_client__harvest_committables(&committables,
@@ -1194,12 +1169,21 @@ svn_client_commit4(svn_commit_info_t **c
      or prop modifications), then we return here to avoid committing a
      revision with no changes. */
   {
-    svn_boolean_t not_found_changed_path = TRUE;
+    svn_boolean_t found_changed_path = FALSE;
+
+    for (i = 0; i < commit_items->nelts; ++i)
+      {
+        svn_client_commit_item3_t *item =
+          APR_ARRAY_IDX(commit_items, i, svn_client_commit_item3_t *);
+
+        if (item->state_flags != SVN_CLIENT_COMMIT_ITEM_LOCK_TOKEN)
+          {
+            found_changed_path = TRUE;
+            break;
+          }
+      }
 
-    cmt_err = svn_iter_apr_array(&not_found_changed_path,
-                                 commit_items,
-                                 commit_item_is_changed, NULL, pool);
-    if (not_found_changed_path || cmt_err)
+    if (!found_changed_path)
       goto cleanup;
   }
 
@@ -1248,23 +1232,24 @@ svn_client_commit4(svn_commit_info_t **c
       || (cmt_err->apr_err == SVN_ERR_REPOS_POST_COMMIT_HOOK_FAILED))
     {
       svn_wc_committed_queue_t *queue = svn_wc_committed_queue_create(pool);
-      struct post_commit_baton btn;
-
-      btn.queue = queue;
-      btn.qpool = pool;
-      btn.wc_ctx = ctx->wc_ctx;
-      btn.keep_changelists = keep_changelists;
-      btn.keep_locks = keep_locks;
-      btn.checksums = checksums;
+      apr_pool_t *iterpool = svn_pool_create(pool);
 
       /* Make a note that our commit is finished. */
       commit_in_progress = FALSE;
 
-      bump_err = svn_iter_apr_array(NULL, commit_items,
-                                    post_process_commit_item, &btn,
-                                    pool);
-      if (bump_err)
-        goto cleanup;
+      for (i = 0; i < commit_items->nelts; i++)
+        {
+          svn_client_commit_item3_t *item
+            = APR_ARRAY_IDX(commit_items, i, svn_client_commit_item3_t *);
+
+          svn_pool_clear(iterpool);
+          bump_err = post_process_commit_item(queue, item, ctx->wc_ctx,
+                                              keep_changelists, keep_locks,
+                                              checksums, pool, iterpool);
+          if (bump_err)
+            goto cleanup;
+        }
+      svn_pool_destroy(iterpool);
 
       SVN_ERR_ASSERT(*commit_info_p);
       bump_err



Mime
View raw message