Merge branch 'svn-fe' of git://repo.or.cz/git/jrn

* 'svn-fe' of git://repo.or.cz/git/jrn:
  vcs-svn: handle log message with embedded NUL
  vcs-svn: avoid unnecessary copying of log message and author
  vcs-svn: remove buffer_read_string
  vcs-svn: make reading of properties binary-safe

8 files changed, 39 insertions, 55 deletions

diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c
index a4d4d9993..2e5bb6725 100644
--- a/vcs-svn/fast_export.c
+++ b/vcs-svn/fast_export.c
@@ -31,12 +31,14 @@ void fast_export_modify(uint32_t depth, uint32_t *path, uint32_t mode,
 }
 
 static char gitsvnline[MAX_GITSVN_LINE_LEN];
-void fast_export_commit(uint32_t revision, const char *author, char *log,
+void fast_export_commit(uint32_t revision, const char *author,
+			const struct strbuf *log,
 			const char *uuid, const char *url,
 			unsigned long timestamp)
 {
+	static const struct strbuf empty = STRBUF_INIT;
 	if (!log)
-		log = "";
+		log = ∅
 	if (*uuid && *url) {
 		snprintf(gitsvnline, MAX_GITSVN_LINE_LEN,
 				"\n\ngit-svn-id: %s@%"PRIu32" %s\n",
@@ -49,9 +51,9 @@ void fast_export_commit(uint32_t revision, const char *author, char *log,
 		   *author ? author : "nobody",
 		   *author ? author : "nobody",
 		   *uuid ? uuid : "local", timestamp);
-	printf("data %"PRIu32"\n%s%s\n",
-		   (uint32_t) (strlen(log) + strlen(gitsvnline)),
-		   log, gitsvnline);
+	printf("data %"PRIuMAX"\n", log->len + strlen(gitsvnline));
+	fwrite(log->buf, log->len, 1, stdout);
+	printf("%s\n", gitsvnline);
 	if (!first_commit_done) {
 		if (revision > 1)
 			printf("from refs/heads/master^0\n");
diff --git a/vcs-svn/fast_export.h b/vcs-svn/fast_export.h
index 05cf97f3a..33a8fe996 100644
--- a/vcs-svn/fast_export.h
+++ b/vcs-svn/fast_export.h
@@ -2,13 +2,14 @@
 #define FAST_EXPORT_H_
 
 #include "line_buffer.h"
+struct strbuf;
 
 void fast_export_delete(uint32_t depth, uint32_t *path);
 void fast_export_modify(uint32_t depth, uint32_t *path, uint32_t mode,
 			uint32_t mark);
-void fast_export_commit(uint32_t revision, const char *author, char *log,
-			const char *uuid, const char *url,
-			unsigned long timestamp);
+void fast_export_commit(uint32_t revision, const char *author,
+			const struct strbuf *log, const char *uuid,
+			const char *url, unsigned long timestamp);
 void fast_export_blob(uint32_t mode, uint32_t mark, uint32_t len,
 		      struct line_buffer *input);
 
diff --git a/vcs-svn/line_buffer.c b/vcs-svn/line_buffer.c
index 33e733a04..c39038723 100644
--- a/vcs-svn/line_buffer.c
+++ b/vcs-svn/line_buffer.c
@@ -91,13 +91,6 @@ char *buffer_read_line(struct line_buffer *buf)
 	return buf->line_buffer;
 }
 
-char *buffer_read_string(struct line_buffer *buf, uint32_t len)
-{
-	strbuf_reset(&buf->blob_buffer);
-	strbuf_fread(&buf->blob_buffer, len, buf->infile);
-	return ferror(buf->infile) ? NULL : buf->blob_buffer.buf;
-}
-
 void buffer_read_binary(struct line_buffer *buf,
 				struct strbuf *sb, uint32_t size)
 {
@@ -134,5 +127,4 @@ off_t buffer_skip_bytes(struct line_buffer *buf, off_t nbytes)
 
 void buffer_reset(struct line_buffer *buf)
 {
-	strbuf_release(&buf->blob_buffer);
 }
diff --git a/vcs-svn/line_buffer.h b/vcs-svn/line_buffer.h
index f5c468afa..d0b22dda7 100644
--- a/vcs-svn/line_buffer.h
+++ b/vcs-svn/line_buffer.h
@@ -7,10 +7,9 @@
 
 struct line_buffer {
 	char line_buffer[LINE_BUFFER_LEN];
-	struct strbuf blob_buffer;
 	FILE *infile;
 };
-#define LINE_BUFFER_INIT {"", STRBUF_INIT, NULL}
+#define LINE_BUFFER_INIT { "", NULL }
 
 int buffer_init(struct line_buffer *buf, const char *filename);
 int buffer_fdinit(struct line_buffer *buf, int fd);
@@ -23,7 +22,6 @@ long buffer_tmpfile_prepare_to_read(struct line_buffer *buf);
 
 int buffer_ferror(struct line_buffer *buf);
 char *buffer_read_line(struct line_buffer *buf);
-char *buffer_read_string(struct line_buffer *buf, uint32_t len);
 int buffer_read_char(struct line_buffer *buf);
 void buffer_read_binary(struct line_buffer *buf, struct strbuf *sb, uint32_t len);
 /* Returns number of bytes read (not necessarily written). */
diff --git a/vcs-svn/line_buffer.txt b/vcs-svn/line_buffer.txt
index 4ef0755cf..8e139eb22 100644
--- a/vcs-svn/line_buffer.txt
+++ b/vcs-svn/line_buffer.txt
@@ -16,8 +16,8 @@ The calling program:
 
  - initializes a `struct line_buffer` to LINE_BUFFER_INIT
  - specifies a file to read with `buffer_init`
- - processes input with `buffer_read_line`, `buffer_read_string`,
-   `buffer_skip_bytes`, and `buffer_copy_bytes`
+ - processes input with `buffer_read_line`, `buffer_skip_bytes`,
+   and `buffer_copy_bytes`
  - closes the file with `buffer_deinit`, perhaps to start over and
    read another file.
 
@@ -37,7 +37,7 @@ the calling program.  A program
    the temporary file
  - declares writing is over with `buffer_tmpfile_prepare_to_read`
  - can re-read what was written with `buffer_read_line`,
-   `buffer_read_string`, and so on
+   `buffer_copy_bytes`, and so on
  - can reuse the temporary file by calling `buffer_tmpfile_rewind`
    again
  - removes the temporary file with `buffer_deinit`, perhaps to
@@ -64,12 +64,6 @@ Functions
 	Read a line and strip off the trailing newline.
 	On failure or end of file, returns NULL.
 
-`buffer_read_string`::
-	Read `len` characters of input or up to the end of the
-	file, whichever comes first.  Returns NULL on error.
-	Returns whatever characters were read (possibly "")
-	for end of file.
-
 `buffer_copy_bytes`::
 	Read `len` bytes of input and dump them to the standard output
 	stream.  Returns early for error or end of file.
diff --git a/vcs-svn/repo_tree.c b/vcs-svn/repo_tree.c
index 632dbd873..a21d89de9 100644
--- a/vcs-svn/repo_tree.c
+++ b/vcs-svn/repo_tree.c
@@ -278,8 +278,9 @@ void repo_diff(uint32_t r1, uint32_t r2)
 		    repo_commit_root_dir(commit_pointer(r2)));
 }
 
-void repo_commit(uint32_t revision, const char *author, char *log,
-		 const char *uuid, const char *url, unsigned long timestamp)
+void repo_commit(uint32_t revision, const char *author,
+		const struct strbuf *log, const char *uuid, const char *url,
+		unsigned long timestamp)
 {
 	fast_export_commit(revision, author, log, uuid, url, timestamp);
 	dent_commit();
diff --git a/vcs-svn/repo_tree.h b/vcs-svn/repo_tree.h
index a1b0e8765..37bde2e37 100644
--- a/vcs-svn/repo_tree.h
+++ b/vcs-svn/repo_tree.h
@@ -1,7 +1,7 @@
 #ifndef REPO_TREE_H_
 #define REPO_TREE_H_
 
-#include "git-compat-util.h"
+struct strbuf;
 
 #define REPO_MODE_DIR 0040000
 #define REPO_MODE_BLB 0100644
@@ -18,7 +18,7 @@ uint32_t repo_read_path(const uint32_t *path);
 uint32_t repo_read_mode(const uint32_t *path);
 void repo_delete(uint32_t *path);
 void repo_commit(uint32_t revision, const char *author,
-		char *log, const char *uuid, const char *url,
+		const struct strbuf *log, const char *uuid, const char *url,
 		long unsigned timestamp);
 void repo_diff(uint32_t r1, uint32_t r2);
 void repo_init(void);
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index ea5b128e4..eef49ca19 100644
--- a/vcs-svn/svndump.c
+++ b/vcs-svn/svndump.c
@@ -83,7 +83,7 @@ static void reset_dump_ctx(const char *url)
 }
 
 static void handle_property(const struct strbuf *key_buf,
-				const char *val, uint32_t len,
+				struct strbuf *val,
 				uint32_t *type_set)
 {
 	const char *key = key_buf->buf;
@@ -95,23 +95,23 @@ static void handle_property(const struct strbuf *key_buf,
 			break;
 		if (!val)
 			die("invalid dump: unsets svn:log");
-		strbuf_reset(&rev_ctx.log);
-		strbuf_add(&rev_ctx.log, val, len);
+		strbuf_swap(&rev_ctx.log, val);
 		break;
 	case sizeof("svn:author"):
 		if (constcmp(key, "svn:author"))
 			break;
-		strbuf_reset(&rev_ctx.author);
-		if (val)
-			strbuf_add(&rev_ctx.author, val, len);
+		if (!val)
+			strbuf_reset(&rev_ctx.author);
+		else
+			strbuf_swap(&rev_ctx.author, val);
 		break;
 	case sizeof("svn:date"):
 		if (constcmp(key, "svn:date"))
 			break;
 		if (!val)
 			die("invalid dump: unsets svn:date");
-		if (parse_date_basic(val, &rev_ctx.timestamp, NULL))
-			warning("invalid timestamp: %s", val);
+		if (parse_date_basic(val->buf, &rev_ctx.timestamp, NULL))
+			warning("invalid timestamp: %s", val->buf);
 		break;
 	case sizeof("svn:executable"):
 	case sizeof("svn:special"):
@@ -147,6 +147,7 @@ static void die_short_read(void)
 static void read_props(void)
 {
 	static struct strbuf key = STRBUF_INIT;
+	static struct strbuf val = STRBUF_INIT;
 	const char *t;
 	/*
 	 * NEEDSWORK: to support simple mode changes like
@@ -163,15 +164,15 @@ static void read_props(void)
 	uint32_t type_set = 0;
 	while ((t = buffer_read_line(&input)) && strcmp(t, "PROPS-END")) {
 		uint32_t len;
-		const char *val;
 		const char type = t[0];
 		int ch;
 
 		if (!type || t[1] != ' ')
 			die("invalid property line: %s\n", t);
 		len = atoi(&t[2]);
-		val = buffer_read_string(&input, len);
-		if (!val || strlen(val) != len)
+		strbuf_reset(&val);
+		buffer_read_binary(&input, &val, len);
+		if (val.len < len)
 			die_short_read();
 
 		/* Discard trailing newline. */
@@ -179,22 +180,17 @@ static void read_props(void)
 		if (ch == EOF)
 			die_short_read();
 		if (ch != '\n')
-			die("invalid dump: expected newline after %s", val);
+			die("invalid dump: expected newline after %s", val.buf);
 
 		switch (type) {
 		case 'K':
+			strbuf_swap(&key, &val);
+			continue;
 		case 'D':
-			strbuf_reset(&key);
-			if (val)
-				strbuf_add(&key, val, len);
-			if (type == 'K')
-				continue;
-			assert(type == 'D');
-			val = NULL;
-			len = 0;
-			/* fall through */
+			handle_property(&val, NULL, &type_set);
+			continue;
 		case 'V':
-			handle_property(&key, val, len, &type_set);
+			handle_property(&key, &val, &type_set);
 			strbuf_reset(&key);
 			continue;
 		default:
@@ -278,7 +274,7 @@ static void handle_revision(void)
 {
 	if (rev_ctx.revision)
 		repo_commit(rev_ctx.revision, rev_ctx.author.buf,
-			rev_ctx.log.buf, dump_ctx.uuid.buf, dump_ctx.url.buf,
+			&rev_ctx.log, dump_ctx.uuid.buf, dump_ctx.url.buf,
 			rev_ctx.timestamp);
 }