Re: [PATCH 1/9] vcs-svn: pass paths through to fast-import
From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:50:49
Hi, David Barr wrote:
vcs-svn/fast_export.c | 47 ++++++++++++++++++------------------ vcs-svn/fast_export.h | 9 +++---- vcs-svn/repo_tree.c | 20 +++++++------- vcs-svn/repo_tree.h | 13 ++++------ vcs-svn/svndump.c | 63 +++++++++++++++++++++---------------------------- 5 files changed, 70 insertions(+), 82 deletions(-)
Hoorah! Simpler and more idiomatic.
quoted hunk ↗ jump to hunk
+++ b/vcs-svn/fast_export.c@@ -32,30 +34,30 @@ void fast_export_reset(void)
[...]
buffer_reset(&report_buffer);
}
-void fast_export_delete(uint32_t depth, const uint32_t *path)
+void fast_export_delete(const char *path)
{
- printf("D \"");
- pool_print_seq_q(depth, path, '/', stdout);
- printf("\"\n");
+ putchar('D');
+ putchar(' ');
+ quote_c_style(path, NULL, stdout, 0);
+ putchar('\n');
}Functional change: if the path doesn't need quoting, this won't surround it with quotation marks. Luckily fast-import doesn't mind. [...]
- printf("M %06"PRIo32" %s \"", mode, dataref);
- pool_print_seq_q(depth, path, '/', stdout);
- printf("\"\n");
+ printf("M %06"PRIo32" %s ", mode, dataref);
+ quote_c_style(path, NULL, stdout, 0);
+ putchar('\n');[...]
- printf("ls :%"PRIu32" \"", rev);
- pool_print_seq_q(depth, path, '/', stdout);
- printf("\"\n");
+ printf("ls :%"PRIu32" ", rev);
+ quote_c_style(path, NULL, stdout, 0);
+ putchar('\n');Likewise. [...]
-static void ls_from_active_commit(uint32_t depth, const uint32_t *path)
+static void ls_from_active_commit(const char *path)
{
/* ls "path/to/file" */
printf("ls \"");
- pool_print_seq_q(depth, path, '/', stdout);
+ quote_c_style(path, NULL, stdout, 1);
printf("\"\n");Single-argument 'ls': quotes always present. Phew. [...]
quoted hunk ↗ jump to hunk
--- a/vcs-svn/repo_tree.h +++ b/vcs-svn/repo_tree.h@@ -8,15 +8,12 @@ #define REPO_MODE_EXE 0100755 #define REPO_MODE_LNK 0120000 -#define REPO_MAX_PATH_LEN 4096 -#define REPO_MAX_PATH_DEPTH 1000
Yes.
quoted hunk ↗ jump to hunk
--- a/vcs-svn/svndump.c +++ b/vcs-svn/svndump.c@@ -11,8 +11,8 @@ #include "repo_tree.h" #include "fast_export.h" #include "line_buffer.h" -#include "obj_pool.h" #include "string_pool.h" +#include "strbuf.h" #define REPORT_FILENO 3@@ -31,32 +31,20 @@ #define LENGTH_UNKNOWN (~0) #define DATE_RFC2822_LEN 31 -/* Create memory pool for log messages */ -obj_pool_gen(log, char, 4096) -
Not a path. :) Snuck in from a separate patch?
static struct line_buffer input = LINE_BUFFER_INIT;
#define REPORT_FILENO 3
-static char *log_copy(uint32_t length, const char *log)
-{[...]
-}
Likewise. [...]
static struct {
uint32_t revision, author;
unsigned long timestamp;
- char *log;
+ struct strbuf log;
} rev_ctx;Likewise. [... etc ...]
quoted hunk ↗ jump to hunk
@@ -406,6 +395,9 @@ int svndump_init(const char *filename) if (buffer_init(&input, filename)) return error("cannot open %s: %s", filename, strerror(errno)); fast_export_init(REPORT_FILENO); + strbuf_init(&rev_ctx.log, 4096); + strbuf_init(&node_ctx.src, 4096); + strbuf_init(&node_ctx.dst, 4096);
4096 because PATH_MAX or some other reason?
quoted hunk ↗ jump to hunk
@@ -415,11 +407,13 @@ int svndump_init(const char *filename) void svndump_deinit(void) { - log_reset(); fast_export_deinit(); reset_dump_ctx(~0); reset_rev_ctx(0); reset_node_ctx(NULL); + strbuf_release(&rev_ctx.log); + strbuf_release(&node_ctx.src); + strbuf_release(&node_ctx.dst);
Side note: it's often not clear what should go in the "prepare for next user" routine and what should go in the "shutting down for good". I suppose these should use strbuf_reset and the memory would be finally freed in svndump_reset? Does it make sense to have two distinct routines like this without a user to demonstrate the trade-offs? Except as noted above, Reviewed-by: Jonathan Nieder <redacted> Thanks; I like where this is going.