You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ju...@apache.org on 2015/01/20 19:20:18 UTC

svn commit: r1653326 - in /subversion/trunk/subversion: include/private/svn_repos_private.h libsvn_repos/dump.c svndumpfilter/svndumpfilter.c svnrdump/dump_editor.c

Author: julianfoad
Date: Tue Jan 20 18:20:18 2015
New Revision: 1653326

URL: http://svn.apache.org/r1653326
Log:
More work on deduplicating dump/load code. This time, the target is the
functionality of formatting and writing headers, rather than significant
amounts of code. Factor this out of svnadmin, svnrdump and svndumpfilter.

This starts to make the writing of headers a batch action rather than a
series of sporadic writes to the stream from various code paths. In turn,
this paves the way for further refactoring.

No functional change.

* subversion/include/private/svn_repos_private.h,
  subversion/libsvn_repos/dump.c
  (svn_repos__dumpfile_headers_create,
   svn_repos__dumpfile_header_push,
   svn_repos__dumpfile_header_pushf,
   svn_repos__dump_headers): New functions.
  (dump_node): Use them instead of writing directly to the stream.

* subversion/svndumpfilter/svndumpfilter.c
  (node_baton_t): Change the type of the headers accumulator. Add a per-node
    pool.
  (new_node_record): Initialize the new fields. Use the new functions
    instead of writing directly to the stream. Don't write the headers to
    the stream here; instead leave them in the node baton for writing later.
  (output_node_record): Rename from 'output_node'. Take separate parameters
    instead of the node baton. Use the new functions instead of writing
    directly to the stream.
  (set_fulltext,
   close_node): Track the changes to output_node.

* subversion/svnrdump/dump_editor.c
  (get_props_content
   dump_node
   dump_mkdir
   dump_pending
   close_file): Use the new style for writing headers.

Modified:
    subversion/trunk/subversion/include/private/svn_repos_private.h
    subversion/trunk/subversion/libsvn_repos/dump.c
    subversion/trunk/subversion/svndumpfilter/svndumpfilter.c
    subversion/trunk/subversion/svnrdump/dump_editor.c

Modified: subversion/trunk/subversion/include/private/svn_repos_private.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_repos_private.h?rev=1653326&r1=1653325&r2=1653326&view=diff
==============================================================================
--- subversion/trunk/subversion/include/private/svn_repos_private.h (original)
+++ subversion/trunk/subversion/include/private/svn_repos_private.h Tue Jan 20 18:20:18 2015
@@ -293,6 +293,51 @@ svn_repos__adjust_mergeinfo_property(svn
                                      apr_pool_t *result_pool,
                                      apr_pool_t *scratch_pool);
 
+/* A header entry.
+ *
+ * (The headers are currently declared here to be of type apr_array_header_t
+ * with svn_repos__dumpfile_header_entry_t entries, but the types could
+ * instead be made opaque.)
+ */
+typedef struct svn_repos__dumpfile_header_entry_t {
+  const char *key, *val;
+} svn_repos__dumpfile_header_entry_t;
+
+/* Create an empty set of headers.
+ */
+apr_array_header_t *
+svn_repos__dumpfile_headers_create(apr_pool_t *pool);
+
+/* Push the header (KEY, VAL) onto HEADERS.
+ *
+ * Duplicate the key and value into HEADERS's pool.
+ */
+void
+svn_repos__dumpfile_header_push(apr_array_header_t *headers,
+                                const char *key,
+                                const char *val);
+
+/* Push the header (KEY, val = VAL_FMT ...) onto HEADERS.
+ *
+ * Duplicate the key and value into HEADERS's pool.
+ */
+void
+svn_repos__dumpfile_header_pushf(apr_array_header_t *headers,
+                                 const char *key,
+                                 const char *val_fmt,
+                                 ...)
+        __attribute__((format(printf, 3, 4)));
+
+/* Write to STREAM the headers in HEADERS followed by a blank line.
+ *
+ * HEADERS is an array of struct {const char *key, *val;}.
+ */
+svn_error_t *
+svn_repos__dump_headers(svn_stream_t *stream,
+                        apr_array_header_t *headers,
+                        svn_boolean_t terminate,
+                        apr_pool_t *scratch_pool);
+
 /* Write a revision record to DUMP_STREAM for revision REVISION with revision
  * properies REVPROPS, creating appropriate headers.
  *

Modified: subversion/trunk/subversion/libsvn_repos/dump.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/dump.c?rev=1653326&r1=1653325&r2=1653326&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_repos/dump.c (original)
+++ subversion/trunk/subversion/libsvn_repos/dump.c Tue Jan 20 18:20:18 2015
@@ -445,6 +445,67 @@ write_revision_headers(svn_stream_t *str
   return SVN_NO_ERROR;
 }
 
+apr_array_header_t *
+svn_repos__dumpfile_headers_create(apr_pool_t *pool)
+{
+  apr_array_header_t *headers
+    = apr_array_make(pool, 5, sizeof(svn_repos__dumpfile_header_entry_t));
+
+  return headers;
+}
+
+void
+svn_repos__dumpfile_header_push(apr_array_header_t *headers,
+                                const char *key,
+                                const char *val)
+{
+  svn_repos__dumpfile_header_entry_t *h
+    = &APR_ARRAY_PUSH(headers, svn_repos__dumpfile_header_entry_t);
+
+  h->key = apr_pstrdup(headers->pool, key);
+  h->val = apr_pstrdup(headers->pool, val);
+}
+
+void
+svn_repos__dumpfile_header_pushf(apr_array_header_t *headers,
+                                 const char *key,
+                                 const char *val_fmt,
+                                 ...)
+{
+  va_list ap;
+  svn_repos__dumpfile_header_entry_t *h
+    = &APR_ARRAY_PUSH(headers, svn_repos__dumpfile_header_entry_t);
+
+  h->key = apr_pstrdup(headers->pool, key);
+  va_start(ap, val_fmt);
+  h->val = apr_pvsprintf(headers->pool, val_fmt, ap);
+  va_end(ap);
+}
+
+svn_error_t *
+svn_repos__dump_headers(svn_stream_t *stream,
+                        apr_array_header_t *headers,
+                        svn_boolean_t terminate,
+                        apr_pool_t *scratch_pool)
+{
+  int i;
+
+  for (i = 0; i < headers->nelts; i++)
+    {
+      svn_repos__dumpfile_header_entry_t *h
+        = &APR_ARRAY_IDX(headers, i, svn_repos__dumpfile_header_entry_t);
+
+      SVN_ERR(svn_stream_printf(stream, scratch_pool,
+                                "%s: %s\n", h->key, h->val));
+    }
+
+  /* End of headers */
+  if (terminate)
+    SVN_ERR(svn_stream_puts(stream, "\n"));
+
+  return SVN_NO_ERROR;
+}
+
 svn_error_t *
 svn_repos__dump_revision_record(svn_stream_t *dump_stream,
                                 svn_revnum_t revision,
@@ -967,6 +1028,7 @@ dump_node(struct edit_baton *eb,
   svn_revnum_t compare_rev = eb->current_rev - 1;
   svn_fs_root_t *compare_root = NULL;
   apr_file_t *delta_file = NULL;
+  apr_array_header_t *headers = svn_repos__dumpfile_headers_create(pool);
 
   /* Maybe validate the path. */
   if (eb->verify || eb->notify_func)
@@ -995,15 +1057,14 @@ dump_node(struct edit_baton *eb,
     }
 
   /* Write out metadata headers for this file node. */
-  SVN_ERR(svn_stream_printf(eb->stream, pool,
-                            SVN_REPOS_DUMPFILE_NODE_PATH ": %s\n",
-                            path));
+  svn_repos__dumpfile_header_push(
+    headers, SVN_REPOS_DUMPFILE_NODE_PATH, path);
   if (kind == svn_node_file)
-    SVN_ERR(svn_stream_puts(eb->stream,
-                            SVN_REPOS_DUMPFILE_NODE_KIND ": file\n"));
+    svn_repos__dumpfile_header_push(
+      headers, SVN_REPOS_DUMPFILE_NODE_KIND, "file");
   else if (kind == svn_node_dir)
-    SVN_ERR(svn_stream_puts(eb->stream,
-                            SVN_REPOS_DUMPFILE_NODE_KIND ": dir\n"));
+    svn_repos__dumpfile_header_push(
+      headers, SVN_REPOS_DUMPFILE_NODE_KIND, "dir");
 
   /* Remove leading slashes from copyfrom paths. */
   if (cmp_path)
@@ -1023,8 +1084,8 @@ dump_node(struct edit_baton *eb,
                   apr_psprintf(pool, _("Change invalid path '%s' in r%ld"),
                                path, eb->current_rev));
 
-      SVN_ERR(svn_stream_puts(eb->stream,
-                              SVN_REPOS_DUMPFILE_NODE_ACTION ": change\n"));
+      svn_repos__dumpfile_header_push(
+        headers, SVN_REPOS_DUMPFILE_NODE_ACTION, "change");
 
       /* either the text or props changed, or possibly both. */
       SVN_ERR(svn_fs_revision_root(&compare_root,
@@ -1054,9 +1115,8 @@ dump_node(struct edit_baton *eb,
             tracker_path_replace(eb->path_tracker, path);
 
           /* a simple delete+add, implied by a single 'replace' action. */
-          SVN_ERR(svn_stream_puts(eb->stream,
-                                  SVN_REPOS_DUMPFILE_NODE_ACTION
-                                  ": replace\n"));
+          svn_repos__dumpfile_header_push(
+            headers, SVN_REPOS_DUMPFILE_NODE_ACTION, "replace");
 
           /* definitely need to dump all content for a replace. */
           if (kind == svn_node_file)
@@ -1083,9 +1143,12 @@ dump_node(struct edit_baton *eb,
 
           /* the path & kind headers have already been printed;  just
              add a delete action, and end the current record.*/
-          SVN_ERR(svn_stream_puts(eb->stream,
-                                  SVN_REPOS_DUMPFILE_NODE_ACTION
-                                  ": delete\n\n"));
+          svn_repos__dumpfile_header_push(
+            headers, SVN_REPOS_DUMPFILE_NODE_ACTION, "delete");
+
+          SVN_ERR(svn_repos__dump_headers(eb->stream, headers, TRUE, pool));
+          /* ### Unusually, here we end this node record with only a single
+                 blank line after the header block -- no extra blank line. */
 
           /* recurse:  print an additional add-with-history record. */
           SVN_ERR(dump_node(eb, path, kind, svn_node_action_add,
@@ -1093,10 +1156,7 @@ dump_node(struct edit_baton *eb,
 
           /* we can leave this routine quietly now, don't need to dump
              any content;  that was already done in the second record. */
-          /* ### This fall-through results in writing an extra two blank
-             lines. This has been the case since r842476, pre-1.0. */
-          must_dump_text = FALSE;
-          must_dump_props = FALSE;
+          return SVN_NO_ERROR;
         }
     }
   else if (action == svn_node_action_delete)
@@ -1109,8 +1169,8 @@ dump_node(struct edit_baton *eb,
           tracker_path_delete(eb->path_tracker, path);
         }
 
-      SVN_ERR(svn_stream_puts(eb->stream,
-                              SVN_REPOS_DUMPFILE_NODE_ACTION ": delete\n"));
+      svn_repos__dumpfile_header_push(
+        headers, SVN_REPOS_DUMPFILE_NODE_ACTION, "delete");
 
       /* we can leave this routine quietly now, don't need to dump
          any content. */
@@ -1125,8 +1185,8 @@ dump_node(struct edit_baton *eb,
                                _("Adding already existing path '%s' in r%ld"),
                                path, eb->current_rev));
 
-      SVN_ERR(svn_stream_puts(eb->stream,
-                              SVN_REPOS_DUMPFILE_NODE_ACTION ": add\n"));
+      svn_repos__dumpfile_header_push(
+        headers, SVN_REPOS_DUMPFILE_NODE_ACTION, "add");
 
       if (! is_copy)
         {
@@ -1167,12 +1227,10 @@ dump_node(struct edit_baton *eb,
                 *eb->found_old_reference = TRUE;
             }
 
-          SVN_ERR(svn_stream_printf(eb->stream, pool,
-                                    SVN_REPOS_DUMPFILE_NODE_COPYFROM_REV
-                                    ": %ld\n"
-                                    SVN_REPOS_DUMPFILE_NODE_COPYFROM_PATH
-                                    ": %s\n",
-                                    cmp_rev, cmp_path));
+          svn_repos__dumpfile_header_pushf(
+            headers, SVN_REPOS_DUMPFILE_NODE_COPYFROM_REV, "%ld", cmp_rev);
+          svn_repos__dumpfile_header_push(
+            headers, SVN_REPOS_DUMPFILE_NODE_COPYFROM_PATH, cmp_path);
 
           SVN_ERR(svn_fs_revision_root(&compare_root,
                                        svn_fs_root_fs(eb->fs_root),
@@ -1196,18 +1254,16 @@ dump_node(struct edit_baton *eb,
                                            FALSE, pool));
               hex_digest = svn_checksum_to_cstring(checksum, pool);
               if (hex_digest)
-                SVN_ERR(svn_stream_printf(eb->stream, pool,
-                                      SVN_REPOS_DUMPFILE_TEXT_COPY_SOURCE_MD5
-                                      ": %s\n", hex_digest));
+                svn_repos__dumpfile_header_push(
+                  headers, SVN_REPOS_DUMPFILE_TEXT_COPY_SOURCE_MD5, hex_digest);
 
               SVN_ERR(svn_fs_file_checksum(&checksum, svn_checksum_sha1,
                                            compare_root, compare_path,
                                            FALSE, pool));
               hex_digest = svn_checksum_to_cstring(checksum, pool);
               if (hex_digest)
-                SVN_ERR(svn_stream_printf(eb->stream, pool,
-                                      SVN_REPOS_DUMPFILE_TEXT_COPY_SOURCE_SHA1
-                                      ": %s\n", hex_digest));
+                svn_repos__dumpfile_header_push(
+                  headers, SVN_REPOS_DUMPFILE_TEXT_COPY_SOURCE_SHA1, hex_digest);
             }
         }
     }
@@ -1219,8 +1275,9 @@ dump_node(struct edit_baton *eb,
          then our dumpstream format demands that at a *minimum*, we
          see a lone "PROPS-END" as a divider between text and props
          content within the content-block. */
-      len = 2;
-      return svn_stream_write(eb->stream, "\n\n", &len); /* ### needed? */
+      SVN_ERR(svn_repos__dump_headers(eb->stream, headers, TRUE, pool));
+      len = 1;
+      return svn_stream_write(eb->stream, "\n", &len); /* ### needed? */
     }
 
   /*** Start prepping content to dump... ***/
@@ -1289,8 +1346,8 @@ dump_node(struct edit_baton *eb,
           if (!oldhash)         /* May have been set for normalization check */
             SVN_ERR(svn_fs_node_proplist(&oldhash, compare_root, compare_path,
                                          pool));
-          SVN_ERR(svn_stream_puts(eb->stream,
-                                  SVN_REPOS_DUMPFILE_PROP_DELTA ": true\n"));
+          svn_repos__dumpfile_header_push(
+            headers, SVN_REPOS_DUMPFILE_PROP_DELTA, "true");
         }
       else
         oldhash = apr_hash_make(pool);
@@ -1301,9 +1358,9 @@ dump_node(struct edit_baton *eb,
       SVN_ERR(svn_stream_close(propstream));
       proplen = propstring->len;
       content_length += proplen;
-      SVN_ERR(svn_stream_printf(eb->stream, pool,
-                                SVN_REPOS_DUMPFILE_PROP_CONTENT_LENGTH
-                                ": %" APR_SIZE_T_FMT "\n", proplen));
+      svn_repos__dumpfile_header_pushf(
+        headers, SVN_REPOS_DUMPFILE_PROP_CONTENT_LENGTH,
+        "%" APR_SIZE_T_FMT, proplen);
     }
 
   /* If we are supposed to dump text, write out a text length header
@@ -1321,8 +1378,8 @@ dump_node(struct edit_baton *eb,
              saying our text contents are a delta. */
           SVN_ERR(store_delta(&delta_file, &textlen, compare_root,
                               compare_path, eb->fs_root, path, pool));
-          SVN_ERR(svn_stream_puts(eb->stream,
-                                  SVN_REPOS_DUMPFILE_TEXT_DELTA ": true\n"));
+          svn_repos__dumpfile_header_push(
+            headers, SVN_REPOS_DUMPFILE_TEXT_DELTA, "true");
 
           if (compare_root)
             {
@@ -1331,18 +1388,16 @@ dump_node(struct edit_baton *eb,
                                            FALSE, pool));
               hex_digest = svn_checksum_to_cstring(checksum, pool);
               if (hex_digest)
-                SVN_ERR(svn_stream_printf(eb->stream, pool,
-                                          SVN_REPOS_DUMPFILE_TEXT_DELTA_BASE_MD5
-                                          ": %s\n", hex_digest));
+                svn_repos__dumpfile_header_push(
+                  headers, SVN_REPOS_DUMPFILE_TEXT_DELTA_BASE_MD5, hex_digest);
 
               SVN_ERR(svn_fs_file_checksum(&checksum, svn_checksum_sha1,
                                            compare_root, compare_path,
                                            FALSE, pool));
               hex_digest = svn_checksum_to_cstring(checksum, pool);
               if (hex_digest)
-                SVN_ERR(svn_stream_printf(eb->stream, pool,
-                                      SVN_REPOS_DUMPFILE_TEXT_DELTA_BASE_SHA1
-                                      ": %s\n", hex_digest));
+                svn_repos__dumpfile_header_push(
+                  headers, SVN_REPOS_DUMPFILE_TEXT_DELTA_BASE_SHA1, hex_digest);
             }
         }
       else
@@ -1352,34 +1407,33 @@ dump_node(struct edit_baton *eb,
         }
 
       content_length += textlen;
-      SVN_ERR(svn_stream_printf(eb->stream, pool,
-                                SVN_REPOS_DUMPFILE_TEXT_CONTENT_LENGTH
-                                ": %" SVN_FILESIZE_T_FMT "\n", textlen));
+      svn_repos__dumpfile_header_pushf(
+        headers, SVN_REPOS_DUMPFILE_TEXT_CONTENT_LENGTH,
+        "%" SVN_FILESIZE_T_FMT, textlen);
 
       SVN_ERR(svn_fs_file_checksum(&checksum, svn_checksum_md5,
                                    eb->fs_root, path, FALSE, pool));
       hex_digest = svn_checksum_to_cstring(checksum, pool);
       if (hex_digest)
-        SVN_ERR(svn_stream_printf(eb->stream, pool,
-                                  SVN_REPOS_DUMPFILE_TEXT_CONTENT_MD5
-                                  ": %s\n", hex_digest));
+        svn_repos__dumpfile_header_push(
+          headers, SVN_REPOS_DUMPFILE_TEXT_CONTENT_MD5, hex_digest);
 
       SVN_ERR(svn_fs_file_checksum(&checksum, svn_checksum_sha1,
                                    eb->fs_root, path, FALSE, pool));
       hex_digest = svn_checksum_to_cstring(checksum, pool);
       if (hex_digest)
-        SVN_ERR(svn_stream_printf(eb->stream, pool,
-                                  SVN_REPOS_DUMPFILE_TEXT_CONTENT_SHA1
-                                  ": %s\n", hex_digest));
+        svn_repos__dumpfile_header_push(
+          headers, SVN_REPOS_DUMPFILE_TEXT_CONTENT_SHA1, hex_digest);
     }
 
   /* 'Content-length:' is the last header before we dump the content,
      and is the sum of the text and prop contents lengths.  We write
      this only for the benefit of non-Subversion RFC-822 parsers. */
-  SVN_ERR(svn_stream_printf(eb->stream, pool,
-                            SVN_REPOS_DUMPFILE_CONTENT_LENGTH
-                            ": %" SVN_FILESIZE_T_FMT "\n\n",
-                            content_length));
+  svn_repos__dumpfile_header_pushf(
+    headers, SVN_REPOS_DUMPFILE_CONTENT_LENGTH,
+    "%" SVN_FILESIZE_T_FMT, content_length);
+
+  SVN_ERR(svn_repos__dump_headers(eb->stream, headers, TRUE, pool));
 
   /* Dump property content if we're supposed to do so. */
   if (must_dump_props)

Modified: subversion/trunk/subversion/svndumpfilter/svndumpfilter.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svndumpfilter/svndumpfilter.c?rev=1653326&r1=1653325&r2=1653326&view=diff
==============================================================================
--- subversion/trunk/subversion/svndumpfilter/svndumpfilter.c (original)
+++ subversion/trunk/subversion/svndumpfilter/svndumpfilter.c Tue Jan 20 18:20:18 2015
@@ -278,7 +278,7 @@ struct node_baton_t
   svn_filesize_t tcl;
 
   /* Pointers to dumpfile data. */
-  svn_stringbuf_t *header;
+  apr_array_header_t *headers;
   svn_stringbuf_t *props;
 
   /* Expect deltas? */
@@ -287,6 +287,8 @@ struct node_baton_t
 
   /* We might need the node path in a parse error message. */
   char *node_path;
+
+  apr_pool_t *node_pool;
 };
 
 
@@ -499,6 +501,7 @@ new_node_record(void **node_baton,
   *node_baton = apr_palloc(pool, sizeof(struct node_baton_t));
   nb          = *node_baton;
   nb->rb      = rev_baton;
+  nb->node_pool = pool;
   pb          = nb->rb->pb;
 
   node_path = svn_hash_gets(headers, SVN_REPOS_DUMPFILE_NODE_PATH);
@@ -570,7 +573,7 @@ new_node_record(void **node_baton,
       nb->has_text_delta = FALSE;
       nb->writing_begun = FALSE;
       nb->tcl = tcl ? svn__atoui64(tcl) : 0;
-      nb->header = svn_stringbuf_create_empty(pool);
+      nb->headers = svn_repos__dumpfile_headers_create(pool);
       nb->props = svn_stringbuf_create_empty(pool);
       nb->node_path = apr_pstrdup(pool, node_path);
 
@@ -582,23 +585,20 @@ new_node_record(void **node_baton,
 
       /* A node record is required to begin with 'Node-path', skip the
          leading '/' to match the form used by 'svnadmin dump'. */
-      SVN_ERR(svn_stream_printf(nb->rb->pb->out_stream,
-                                pool, "%s: %s\n",
-                                SVN_REPOS_DUMPFILE_NODE_PATH, node_path + 1));
+      svn_repos__dumpfile_header_push(
+        nb->headers, SVN_REPOS_DUMPFILE_NODE_PATH, node_path + 1);
 
       /* Node-kind is next and is optional. */
       kind = svn_hash_gets(headers, SVN_REPOS_DUMPFILE_NODE_KIND);
       if (kind)
-        SVN_ERR(svn_stream_printf(nb->rb->pb->out_stream,
-                                  pool, "%s: %s\n",
-                                  SVN_REPOS_DUMPFILE_NODE_KIND, kind));
+        svn_repos__dumpfile_header_push(
+          nb->headers, SVN_REPOS_DUMPFILE_NODE_KIND, kind);
 
       /* Node-action is next and required. */
       action = svn_hash_gets(headers, SVN_REPOS_DUMPFILE_NODE_ACTION);
       if (action)
-        SVN_ERR(svn_stream_printf(nb->rb->pb->out_stream,
-                                  pool, "%s: %s\n",
-                                  SVN_REPOS_DUMPFILE_NODE_ACTION, action));
+        svn_repos__dumpfile_header_push(
+          nb->headers, SVN_REPOS_DUMPFILE_NODE_ACTION, action);
       else
         return svn_error_createf(SVN_ERR_INCOMPLETE_DATA, 0,
                                  _("Missing Node-action for path '%s'"),
@@ -645,18 +645,14 @@ new_node_record(void **node_baton,
                 return svn_error_createf
                   (SVN_ERR_NODE_UNEXPECTED_KIND, NULL,
                    _("No valid copyfrom revision in filtered stream"));
-              SVN_ERR(svn_stream_printf
-                      (nb->rb->pb->out_stream, pool,
-                       SVN_REPOS_DUMPFILE_NODE_COPYFROM_REV ": %ld\n",
-                       cf_renum_val->rev));
+              svn_repos__dumpfile_header_pushf(
+                nb->headers, SVN_REPOS_DUMPFILE_NODE_COPYFROM_REV,
+                "%ld", cf_renum_val->rev);
               continue;
             }
 
           /* passthru: put header straight to output */
-
-          SVN_ERR(svn_stream_printf(nb->rb->pb->out_stream,
-                                    pool, "%s: %s\n",
-                                    key, val));
+          svn_repos__dumpfile_header_push(nb->headers, key, val);
         }
     }
 
@@ -664,61 +660,62 @@ new_node_record(void **node_baton,
 }
 
 
-/* Output node header and props to dumpstream
-   This will be called by set_fulltext() after setting nb->has_text to TRUE,
-   if the node has any text, or by close_node() otherwise. This must only
-   be called if nb->writing_begun is FALSE. */
+/* Output node headers and props.
+ *
+ * Write HEADERS, content length headers, a blank line, and the properties
+ * content section PROPS_STR (if non-null) to DUMP_STREAM.
+ *
+ * HEADERS is an array of headers as struct {const char *key, *val;}.
+ * Write them all in the given order.
+ *
+ * PROPS_STR is the property content block, including a terminating
+ * 'PROPS_END\n' line. Iff PROPS_STR is non-null, write a
+ * Prop-content-length header and the prop content block.
+ *
+ * Iff HAS_TEXT is true, write a Text-content length, using the value
+ * TEXT_CONTENT_LENGTH.
+ *
+ * Always write a Content-length header, its value being the sum of the
+ * Prop- and Text- content length headers.
+ * ### TODO: Make it optional (only written if props and/or text present).
+ */
 static svn_error_t *
-output_node(struct node_baton_t *nb)
+output_node_record(svn_stream_t *dump_stream,
+                   apr_array_header_t *headers,
+                   svn_stringbuf_t *props_str,
+                   svn_boolean_t has_text,
+                   svn_filesize_t text_content_length,
+                   apr_pool_t *scratch_pool)
 {
-  int bytes_used;
-  char buf[SVN_KEYLINE_MAXLEN];
-
-  nb->writing_begun = TRUE;
-
-  /* when there are no props nb->props->len would be zero and won't mess up
-     Content-Length. */
-  if (nb->has_props)
-    svn_stringbuf_appendcstr(nb->props, "PROPS-END\n");
+  svn_filesize_t content_length = 0;
 
-  /* 1. recalculate & check text-md5 if present. Passed through right now. */
-
-  /* 2. recalculate and add content-lengths */
-
-  if (nb->has_props)
+  /* add content-length headers */
+  if (props_str)
     {
-      svn_stringbuf_appendcstr(nb->header,
-                               SVN_REPOS_DUMPFILE_PROP_CONTENT_LENGTH);
-      bytes_used = apr_snprintf(buf, sizeof(buf), ": %" APR_SIZE_T_FMT,
-                                nb->props->len);
-      svn_stringbuf_appendbytes(nb->header, buf, bytes_used);
-      svn_stringbuf_appendbyte(nb->header, '\n');
+      svn_repos__dumpfile_header_pushf(
+        headers, SVN_REPOS_DUMPFILE_PROP_CONTENT_LENGTH,
+        "%" APR_SIZE_T_FMT, props_str->len);
+      content_length += props_str->len;
     }
-  if (nb->has_text)
+  if (has_text)
     {
-      svn_stringbuf_appendcstr(nb->header,
-                               SVN_REPOS_DUMPFILE_TEXT_CONTENT_LENGTH);
-      bytes_used = apr_snprintf(buf, sizeof(buf), ": %" SVN_FILESIZE_T_FMT,
-                                nb->tcl);
-      svn_stringbuf_appendbytes(nb->header, buf, bytes_used);
-      svn_stringbuf_appendbyte(nb->header, '\n');
+      svn_repos__dumpfile_header_pushf(
+        headers, SVN_REPOS_DUMPFILE_TEXT_CONTENT_LENGTH,
+        "%" SVN_FILESIZE_T_FMT, text_content_length);
+      content_length += text_content_length;
     }
-  svn_stringbuf_appendcstr(nb->header, SVN_REPOS_DUMPFILE_CONTENT_LENGTH);
-  bytes_used = apr_snprintf(buf, sizeof(buf), ": %" SVN_FILESIZE_T_FMT,
-                            (svn_filesize_t) (nb->props->len + nb->tcl));
-  svn_stringbuf_appendbytes(nb->header, buf, bytes_used);
-  svn_stringbuf_appendbyte(nb->header, '\n');
-
-  /* put an end to headers */
-  svn_stringbuf_appendbyte(nb->header, '\n');
+  svn_repos__dumpfile_header_pushf(
+    headers, SVN_REPOS_DUMPFILE_CONTENT_LENGTH,
+    "%" SVN_FILESIZE_T_FMT, content_length);
 
-  /* 3. output all the stuff */
-
-  SVN_ERR(svn_stream_write(nb->rb->pb->out_stream,
-                           nb->header->data , &(nb->header->len)));
-  SVN_ERR(svn_stream_write(nb->rb->pb->out_stream,
-                           nb->props->data , &(nb->props->len)));
+  /* write the headers */
+  SVN_ERR(svn_repos__dump_headers(dump_stream, headers, TRUE, scratch_pool));
 
+  /* write the props */
+  if (props_str)
+    {
+      SVN_ERR(svn_stream_write(dump_stream, props_str->data, &props_str->len));
+    }
   return SVN_NO_ERROR;
 }
 
@@ -921,7 +918,19 @@ set_fulltext(svn_stream_t **stream, void
     {
       nb->has_text = TRUE;
       if (! nb->writing_begun)
-        SVN_ERR(output_node(nb));
+        {
+          nb->writing_begun = TRUE;
+          if (nb->has_props)
+            {
+              svn_stringbuf_appendcstr(nb->props, "PROPS-END\n");
+            }
+          SVN_ERR(output_node_record(nb->rb->pb->out_stream,
+                                     nb->headers,
+                                     nb->has_props ? nb->props : NULL,
+                                     nb->has_text,
+                                     nb->tcl,
+                                     nb->node_pool));
+        }
       *stream = nb->rb->pb->out_stream;
     }
 
@@ -942,7 +951,19 @@ close_node(void *node_baton)
 
   /* If the node was not flushed already to output its text, do it now. */
   if (! nb->writing_begun)
-    SVN_ERR(output_node(nb));
+    {
+      nb->writing_begun = TRUE;
+      if (nb->has_props)
+        {
+          svn_stringbuf_appendcstr(nb->props, "PROPS-END\n");
+        }
+      SVN_ERR(output_node_record(nb->rb->pb->out_stream,
+                                 nb->headers,
+                                 nb->has_props ? nb->props : NULL,
+                                 nb->has_text,
+                                 nb->tcl,
+                                 nb->node_pool));
+    }
 
   /* put an end to node. */
   SVN_ERR(svn_stream_write(nb->rb->pb->out_stream, "\n\n", &len));

Modified: subversion/trunk/subversion/svnrdump/dump_editor.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnrdump/dump_editor.c?rev=1653326&r1=1653325&r2=1653326&view=diff
==============================================================================
--- subversion/trunk/subversion/svnrdump/dump_editor.c (original)
+++ subversion/trunk/subversion/svnrdump/dump_editor.c Tue Jan 20 18:20:18 2015
@@ -30,6 +30,7 @@
 #include "svn_subst.h"
 #include "svn_dirent_uri.h"
 
+#include "private/svn_repos_private.h"
 #include "private/svn_subr_private.h"
 #include "private/svn_dep_compat.h"
 #include "private/svn_editor.h"
@@ -229,9 +230,11 @@ make_file_baton(const char *path,
   return new_fb;
 }
 
-/* Return in *HEADER and *CONTENT the headers and content for PROPS. */
+/* Append to HEADERS the required headers, and set *CONTENT to the property
+ * content section, to represent the property delta of PROPS/DELETED_PROPS.
+ */
 static svn_error_t *
-get_props_content(svn_stringbuf_t **header,
+get_props_content(apr_array_header_t *headers,
                   svn_stringbuf_t **content,
                   apr_hash_t *props,
                   apr_hash_t *deleted_props,
@@ -240,10 +243,8 @@ get_props_content(svn_stringbuf_t **head
 {
   svn_stream_t *content_stream;
   apr_hash_t *normal_props;
-  const char *buf;
 
   *content = svn_stringbuf_create_empty(result_pool);
-  *header = svn_stringbuf_create_empty(result_pool);
 
   content_stream = svn_stream_from_stringbuf(*content, scratch_pool);
 
@@ -254,13 +255,13 @@ get_props_content(svn_stringbuf_t **head
   SVN_ERR(svn_stream_close(content_stream));
 
   /* Prop-delta: true */
-  *header = svn_stringbuf_createf(result_pool, SVN_REPOS_DUMPFILE_PROP_DELTA
-                                  ": true\n");
+  svn_repos__dumpfile_header_push(
+    headers, SVN_REPOS_DUMPFILE_PROP_DELTA, "true");
 
   /* Prop-content-length: 193 */
-  buf = apr_psprintf(scratch_pool, SVN_REPOS_DUMPFILE_PROP_CONTENT_LENGTH
-                     ": %" APR_SIZE_T_FMT "\n", (*content)->len);
-  svn_stringbuf_appendcstr(*header, buf);
+  svn_repos__dumpfile_header_pushf(
+    headers, SVN_REPOS_DUMPFILE_PROP_CONTENT_LENGTH,
+    "%" APR_SIZE_T_FMT, (*content)->len);
 
   return SVN_NO_ERROR;
 }
@@ -279,7 +280,7 @@ do_dump_newlines(struct dump_edit_baton
 }
 
 /*
- * Write out a node record for PATH of type KIND under EB->FS_ROOT.
+ * Write out a partial node record for PATH of type KIND.
  * ACTION describes what is happening to the node (see enum
  * svn_node_action). Write record to writable EB->STREAM.
  *
@@ -287,6 +288,9 @@ do_dump_newlines(struct dump_edit_baton
  * path/revision of the copy source are in COPYFROM_PATH/COPYFROM_REV.
  * If IS_COPY is FALSE, yet COPYFROM_PATH/COPYFROM_REV are valid, this
  * node is part of a copied subtree.
+
+ * Note: only when ACTION=delete, write a complete dumpfile record
+ * terminated with blank lines.
  */
 static svn_error_t *
 dump_node(struct dump_edit_baton *eb,
@@ -300,6 +304,7 @@ dump_node(struct dump_edit_baton *eb,
           apr_pool_t *pool)
 {
   const char *node_relpath = repos_relpath;
+  apr_array_header_t *headers = svn_repos__dumpfile_headers_create(pool);
 
   assert(svn_relpath_is_canonical(repos_relpath));
   assert(!copyfrom_path || svn_relpath_is_canonical(copyfrom_path));
@@ -311,17 +316,16 @@ dump_node(struct dump_edit_baton *eb,
                                     node_relpath, pool);
 
   /* Node-path: ... */
-  SVN_ERR(svn_stream_printf(eb->stream, pool,
-                            SVN_REPOS_DUMPFILE_NODE_PATH ": %s\n",
-                            node_relpath));
+  svn_repos__dumpfile_header_push(
+    headers, SVN_REPOS_DUMPFILE_NODE_PATH, node_relpath);
 
   /* Node-kind: "file" | "dir" */
   if (fb)
-    SVN_ERR(svn_stream_printf(eb->stream, pool,
-                              SVN_REPOS_DUMPFILE_NODE_KIND ": file\n"));
+    svn_repos__dumpfile_header_push(
+      headers, SVN_REPOS_DUMPFILE_NODE_KIND, "file");
   else if (db)
-    SVN_ERR(svn_stream_printf(eb->stream, pool,
-                              SVN_REPOS_DUMPFILE_NODE_KIND ": dir\n"));
+    svn_repos__dumpfile_header_push(
+      headers, SVN_REPOS_DUMPFILE_NODE_KIND, "dir");
 
 
   /* Write the appropriate Node-action header */
@@ -333,8 +337,8 @@ dump_node(struct dump_edit_baton *eb,
          do here but print node action information.
 
          Node-action: change.  */
-      SVN_ERR(svn_stream_puts(eb->stream,
-                              SVN_REPOS_DUMPFILE_NODE_ACTION ": change\n"));
+      svn_repos__dumpfile_header_push(
+        headers, SVN_REPOS_DUMPFILE_NODE_ACTION, "change");
       break;
 
     case svn_node_action_replace:
@@ -344,20 +348,21 @@ dump_node(struct dump_edit_baton *eb,
              copyfrom_rev are present: delete the original, and then re-add
              it */
 
-          SVN_ERR(svn_stream_puts(eb->stream,
-                                  SVN_REPOS_DUMPFILE_NODE_ACTION
-                                  ": delete\n\n"));
+          svn_repos__dumpfile_header_push(
+            headers, SVN_REPOS_DUMPFILE_NODE_ACTION, "delete");
+
+          SVN_ERR(svn_repos__dump_headers(eb->stream, headers, TRUE, pool));
 
           /* Recurse: Print an additional add-with-history record. */
           SVN_ERR(dump_node(eb, repos_relpath, db, fb, svn_node_action_add,
                             is_copy, copyfrom_path, copyfrom_rev, pool));
+          return SVN_NO_ERROR;
         }
       else
         {
           /* Node-action: replace */
-          SVN_ERR(svn_stream_puts(eb->stream,
-                                  SVN_REPOS_DUMPFILE_NODE_ACTION
-                                  ": replace\n"));
+          svn_repos__dumpfile_header_push(
+            headers, SVN_REPOS_DUMPFILE_NODE_ACTION, "replace");
 
           /* Wait for a change_*_prop to be called before dumping
              anything */
@@ -370,30 +375,29 @@ dump_node(struct dump_edit_baton *eb,
 
     case svn_node_action_delete:
       /* Node-action: delete */
-      SVN_ERR(svn_stream_puts(eb->stream,
-                              SVN_REPOS_DUMPFILE_NODE_ACTION ": delete\n"));
+      svn_repos__dumpfile_header_push(
+        headers, SVN_REPOS_DUMPFILE_NODE_ACTION, "delete");
 
       /* We can leave this routine quietly now. Nothing more to do-
-         print a couple of newlines because we're not dumping props or
-         text. */
-      SVN_ERR(svn_stream_puts(eb->stream, "\n\n"));
+         print the headers terminated by one blank line, and an extra
+         blank line because we're not dumping props or text. */
+      SVN_ERR(svn_repos__dump_headers(eb->stream, headers, TRUE, pool));
+      SVN_ERR(svn_stream_puts(eb->stream, "\n"));
 
-      break;
+      return SVN_NO_ERROR;
 
     case svn_node_action_add:
       /* Node-action: add */
-      SVN_ERR(svn_stream_puts(eb->stream,
-                              SVN_REPOS_DUMPFILE_NODE_ACTION ": add\n"));
+      svn_repos__dumpfile_header_push(
+        headers, SVN_REPOS_DUMPFILE_NODE_ACTION, "add");
 
       if (is_copy)
         {
           /* Node-copyfrom-rev / Node-copyfrom-path */
-          SVN_ERR(svn_stream_printf(eb->stream, pool,
-                                    SVN_REPOS_DUMPFILE_NODE_COPYFROM_REV
-                                    ": %ld\n"
-                                    SVN_REPOS_DUMPFILE_NODE_COPYFROM_PATH
-                                    ": %s\n",
-                                    copyfrom_rev, copyfrom_path));
+          svn_repos__dumpfile_header_pushf(
+            headers, SVN_REPOS_DUMPFILE_NODE_COPYFROM_REV, "%ld", copyfrom_rev);
+          svn_repos__dumpfile_header_push(
+            headers, SVN_REPOS_DUMPFILE_NODE_COPYFROM_PATH, copyfrom_path);
 
           /* Ugly hack: If a directory was copied from a previous
              revision, nothing like close_file() will be called to write two
@@ -431,6 +435,11 @@ dump_node(struct dump_edit_baton *eb,
 
       break;
     }
+
+  /* Write the headers so far. We don't necessarily have all the headers
+     yet -- there may be property-related and content length headers to
+     come -- so don't write a terminating blank line. */
+  SVN_ERR(svn_repos__dump_headers(eb->stream, headers, FALSE, pool));
   return SVN_NO_ERROR;
 }
 
@@ -439,34 +448,30 @@ dump_mkdir(struct dump_edit_baton *eb,
            const char *repos_relpath,
            apr_pool_t *pool)
 {
-  svn_stringbuf_t *prop_header, *prop_content;
+  svn_stringbuf_t *prop_content;
   apr_size_t len;
-  const char *buf;
+  apr_array_header_t *headers = svn_repos__dumpfile_headers_create(pool);
 
   /* Node-path: ... */
-  SVN_ERR(svn_stream_printf(eb->stream, pool,
-                            SVN_REPOS_DUMPFILE_NODE_PATH ": %s\n",
-                            repos_relpath));
+  svn_repos__dumpfile_header_push(
+    headers, SVN_REPOS_DUMPFILE_NODE_PATH, repos_relpath);
 
   /* Node-kind: dir */
-  SVN_ERR(svn_stream_printf(eb->stream, pool,
-                            SVN_REPOS_DUMPFILE_NODE_KIND ": dir\n"));
+  svn_repos__dumpfile_header_push(
+    headers, SVN_REPOS_DUMPFILE_NODE_KIND, "dir");
 
   /* Node-action: add */
-  SVN_ERR(svn_stream_puts(eb->stream,
-                          SVN_REPOS_DUMPFILE_NODE_ACTION ": add\n"));
+  svn_repos__dumpfile_header_push(
+    headers, SVN_REPOS_DUMPFILE_NODE_ACTION, "add");
 
   /* Dump the (empty) property block. */
-  SVN_ERR(get_props_content(&prop_header, &prop_content,
+  SVN_ERR(get_props_content(headers, &prop_content,
                             apr_hash_make(pool), apr_hash_make(pool),
                             pool, pool));
-  len = prop_header->len;
-  SVN_ERR(svn_stream_write(eb->stream, prop_header->data, &len));
   len = prop_content->len;
-  buf = apr_psprintf(pool, SVN_REPOS_DUMPFILE_CONTENT_LENGTH
-                     ": %" APR_SIZE_T_FMT "\n", len);
-  SVN_ERR(svn_stream_puts(eb->stream, buf));
-  SVN_ERR(svn_stream_puts(eb->stream, "\n"));
+  svn_repos__dumpfile_header_pushf(headers, SVN_REPOS_DUMPFILE_CONTENT_LENGTH,
+                                   "%" SVN_FILESIZE_T_FMT, len);
+  SVN_ERR(svn_repos__dump_headers(eb->stream, headers, TRUE, pool));
   SVN_ERR(svn_stream_write(eb->stream, prop_content->data, &len));
 
   /* Newlines to tie it all off. */
@@ -518,19 +523,21 @@ dump_pending(struct dump_edit_baton *eb,
 
   if (dump_props)
     {
-      svn_stringbuf_t *header, *content;
+      apr_array_header_t *headers
+        = svn_repos__dumpfile_headers_create(scratch_pool);
+      svn_stringbuf_t *content;
       apr_size_t len;
 
-      SVN_ERR(get_props_content(&header, &content, props, deleted_props,
+      SVN_ERR(get_props_content(headers, &content, props, deleted_props,
                                 scratch_pool, scratch_pool));
-      len = header->len;
-      SVN_ERR(svn_stream_write(eb->stream, header->data, &len));
 
       /* Content-length: 14 */
-      SVN_ERR(svn_stream_printf(eb->stream, scratch_pool,
-                                SVN_REPOS_DUMPFILE_CONTENT_LENGTH
-                                ": %" APR_SIZE_T_FMT "\n\n",
-                                content->len));
+      svn_repos__dumpfile_header_pushf(
+        headers, SVN_REPOS_DUMPFILE_CONTENT_LENGTH,
+        "%" APR_SIZE_T_FMT, content->len);
+
+      SVN_ERR(svn_repos__dump_headers(eb->stream, headers, TRUE,
+                                      scratch_pool));
 
       len = content->len;
       SVN_ERR(svn_stream_write(eb->stream, content->data, &len));
@@ -953,6 +960,7 @@ close_file(void *file_baton,
   struct dump_edit_baton *eb = fb->eb;
   apr_finfo_t *info = apr_pcalloc(pool, sizeof(apr_finfo_t));
   svn_stringbuf_t *propstring;
+  apr_array_header_t *headers = svn_repos__dumpfile_headers_create(pool);
 
   LDR_DBG(("close_file %p\n", file_baton));
 
@@ -968,13 +976,9 @@ close_file(void *file_baton,
      the text headers too (if present). */
   if (fb->dump_props)
     {
-      svn_stringbuf_t *header;
-      apr_size_t len;
-
-      SVN_ERR(get_props_content(&header, &propstring, fb->props, fb->deleted_props,
+      SVN_ERR(get_props_content(headers, &propstring,
+                                fb->props, fb->deleted_props,
                                 pool, pool));
-      len = header->len;
-      SVN_ERR(svn_stream_write(eb->stream, header->data, &len));
     }
 
   /* Dump the text headers */
@@ -983,9 +987,8 @@ close_file(void *file_baton,
       apr_status_t err;
 
       /* Text-delta: true */
-      SVN_ERR(svn_stream_puts(eb->stream,
-                              SVN_REPOS_DUMPFILE_TEXT_DELTA
-                              ": true\n"));
+      svn_repos__dumpfile_header_push(
+        headers, SVN_REPOS_DUMPFILE_TEXT_DELTA, "true");
 
       err = apr_file_info_get(info, APR_FINFO_SIZE, eb->delta_file);
       if (err)
@@ -993,36 +996,31 @@ close_file(void *file_baton,
 
       if (fb->base_checksum)
         /* Text-delta-base-md5: */
-        SVN_ERR(svn_stream_printf(eb->stream, pool,
-                                  SVN_REPOS_DUMPFILE_TEXT_DELTA_BASE_MD5
-                                  ": %s\n",
-                                  fb->base_checksum));
+        svn_repos__dumpfile_header_push(
+          headers, SVN_REPOS_DUMPFILE_TEXT_DELTA_BASE_MD5, fb->base_checksum);
 
       /* Text-content-length: 39 */
-      SVN_ERR(svn_stream_printf(eb->stream, pool,
-                                SVN_REPOS_DUMPFILE_TEXT_CONTENT_LENGTH
-                                ": %lu\n",
-                                (unsigned long)info->size));
+      svn_repos__dumpfile_header_pushf(
+        headers, SVN_REPOS_DUMPFILE_TEXT_CONTENT_LENGTH,
+        "%lu", (unsigned long)info->size);
 
       /* Text-content-md5: 82705804337e04dcd0e586bfa2389a7f */
-      SVN_ERR(svn_stream_printf(eb->stream, pool,
-                                SVN_REPOS_DUMPFILE_TEXT_CONTENT_MD5
-                                ": %s\n",
-                                text_checksum));
+      svn_repos__dumpfile_header_push(
+        headers, SVN_REPOS_DUMPFILE_TEXT_CONTENT_MD5, text_checksum);
     }
 
   /* Content-length: 1549 */
   /* If both text and props are absent, skip this header */
   if (fb->dump_props)
-    SVN_ERR(svn_stream_printf(eb->stream, pool,
-                              SVN_REPOS_DUMPFILE_CONTENT_LENGTH
-                              ": %ld\n\n",
-                              (unsigned long)info->size + propstring->len));
+    svn_repos__dumpfile_header_pushf(
+      headers, SVN_REPOS_DUMPFILE_CONTENT_LENGTH,
+      "%ld", (unsigned long)info->size + propstring->len);
   else if (fb->dump_text)
-    SVN_ERR(svn_stream_printf(eb->stream, pool,
-                              SVN_REPOS_DUMPFILE_CONTENT_LENGTH
-                              ": %ld\n\n",
-                              (unsigned long)info->size));
+    svn_repos__dumpfile_header_pushf(
+      headers, SVN_REPOS_DUMPFILE_CONTENT_LENGTH,
+      "%ld", (unsigned long)info->size);
+
+  SVN_ERR(svn_repos__dump_headers(eb->stream, headers, TRUE, pool));
 
   /* Dump the props now */
   if (fb->dump_props)