Merge branch 'jc/bindiff'

* jc/bindiff:
  improve base85 generated assembly code
  binary diff and apply: testsuite.
  binary diff: further updates.
  binary patch.

8 files changed, 575 insertions, 69 deletions

diff --git a/Makefile b/Makefile
index 45484fcd8..814010d7b 100644
--- a/Makefile
+++ b/Makefile
@@ -205,7 +205,7 @@ DIFF_OBJS = \
 	diffcore-delta.o log-tree.o
 
 LIB_OBJS = \
-	blob.o commit.o connect.o csum-file.o \
+	blob.o commit.o connect.o csum-file.o base85.o \
 	date.o diff-delta.o entry.o exec_cmd.o ident.o index.o \
 	object.o pack-check.o patch-delta.o path.o pkt-line.o \
 	quote.o read-cache.o refs.o run-command.o \
diff --git a/apply.c b/apply.c
index ca36391bb..7c8146a7f 100644
--- a/apply.c
+++ b/apply.c
@@ -10,6 +10,7 @@
 #include "cache.h"
 #include "quote.h"
 #include "blob.h"
+#include "delta.h"
 
 //  --check turns on checking that the working tree matches the
 //    files that are being modified, but doesn't apply the patch
@@ -114,6 +115,9 @@ struct patch {
 	char *new_name, *old_name, *def_name;
 	unsigned int old_mode, new_mode;
 	int is_rename, is_copy, is_new, is_delete, is_binary;
+#define BINARY_DELTA_DEFLATED 1
+#define BINARY_LITERAL_DEFLATED 2
+	unsigned long deflate_origlen;
 	int lines_added, lines_deleted;
 	int score;
 	struct fragment *fragments;
@@ -967,6 +971,88 @@ static inline int metadata_changes(struct patch *patch)
 		 patch->old_mode != patch->new_mode);
 }
 
+static int parse_binary(char *buffer, unsigned long size, struct patch *patch)
+{
+	/* We have read "GIT binary patch\n"; what follows is a line
+	 * that says the patch method (currently, either "deflated
+	 * literal" or "deflated delta") and the length of data before
+	 * deflating; a sequence of 'length-byte' followed by base-85
+	 * encoded data follows.
+	 *
+	 * Each 5-byte sequence of base-85 encodes up to 4 bytes,
+	 * and we would limit the patch line to 66 characters,
+	 * so one line can fit up to 13 groups that would decode
+	 * to 52 bytes max.  The length byte 'A'-'Z' corresponds
+	 * to 1-26 bytes, and 'a'-'z' corresponds to 27-52 bytes.
+	 * The end of binary is signalled with an empty line.
+	 */
+	int llen, used;
+	struct fragment *fragment;
+	char *data = NULL;
+
+	patch->fragments = fragment = xcalloc(1, sizeof(*fragment));
+
+	/* Grab the type of patch */
+	llen = linelen(buffer, size);
+	used = llen;
+	linenr++;
+
+	if (!strncmp(buffer, "delta ", 6)) {
+		patch->is_binary = BINARY_DELTA_DEFLATED;
+		patch->deflate_origlen = strtoul(buffer + 6, NULL, 10);
+	}
+	else if (!strncmp(buffer, "literal ", 8)) {
+		patch->is_binary = BINARY_LITERAL_DEFLATED;
+		patch->deflate_origlen = strtoul(buffer + 8, NULL, 10);
+	}
+	else
+		return error("unrecognized binary patch at line %d: %.*s",
+			     linenr-1, llen-1, buffer);
+	buffer += llen;
+	while (1) {
+		int byte_length, max_byte_length, newsize;
+		llen = linelen(buffer, size);
+		used += llen;
+		linenr++;
+		if (llen == 1)
+			break;
+		/* Minimum line is "A00000\n" which is 7-byte long,
+		 * and the line length must be multiple of 5 plus 2.
+		 */
+		if ((llen < 7) || (llen-2) % 5)
+			goto corrupt;
+		max_byte_length = (llen - 2) / 5 * 4;
+		byte_length = *buffer;
+		if ('A' <= byte_length && byte_length <= 'Z')
+			byte_length = byte_length - 'A' + 1;
+		else if ('a' <= byte_length && byte_length <= 'z')
+			byte_length = byte_length - 'a' + 27;
+		else
+			goto corrupt;
+		/* if the input length was not multiple of 4, we would
+		 * have filler at the end but the filler should never
+		 * exceed 3 bytes
+		 */
+		if (max_byte_length < byte_length ||
+		    byte_length <= max_byte_length - 4)
+			goto corrupt;
+		newsize = fragment->size + byte_length;
+		data = xrealloc(data, newsize);
+		if (decode_85(data + fragment->size,
+			      buffer + 1,
+			      byte_length))
+			goto corrupt;
+		fragment->size = newsize;
+		buffer += llen;
+		size -= llen;
+	}
+	fragment->patch = data;
+	return used;
+ corrupt:
+	return error("corrupt binary patch at line %d: %.*s",
+		     linenr-1, llen-1, buffer);
+}
+
 static int parse_chunk(char *buffer, unsigned long size, struct patch *patch)
 {
 	int hdrsize, patchsize;
@@ -983,19 +1069,34 @@ static int parse_chunk(char *buffer, unsigned long size, struct patch *patch)
 			"Files ",
 			NULL,
 		};
+		static const char git_binary[] = "GIT binary patch\n";
 		int i;
 		int hd = hdrsize + offset;
 		unsigned long llen = linelen(buffer + hd, size - hd);
 
-		if (!memcmp(" differ\n", buffer + hd + llen - 8, 8))
+		if (llen == sizeof(git_binary) - 1 &&
+		    !memcmp(git_binary, buffer + hd, llen)) {
+			int used;
+			linenr++;
+			used = parse_binary(buffer + hd + llen,
+					    size - hd - llen, patch);
+			if (used)
+				patchsize = used + llen;
+			else
+				patchsize = 0;
+		}
+		else if (!memcmp(" differ\n", buffer + hd + llen - 8, 8)) {
 			for (i = 0; binhdr[i]; i++) {
 				int len = strlen(binhdr[i]);
 				if (len < size - hd &&
 				    !memcmp(binhdr[i], buffer + hd, len)) {
+					linenr++;
 					patch->is_binary = 1;
+					patchsize = llen;
 					break;
 				}
 			}
+		}
 
 		/* Empty patch cannot be applied if:
 		 * - it is a binary patch and we do not do binary_replace, or
@@ -1346,76 +1447,150 @@ static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag)
 	return offset;
 }
 
-static int apply_fragments(struct buffer_desc *desc, struct patch *patch)
+static char *inflate_it(const void *data, unsigned long size,
+			unsigned long inflated_size)
+{
+	z_stream stream;
+	void *out;
+	int st;
+
+	memset(&stream, 0, sizeof(stream));
+
+	stream.next_in = (unsigned char *)data;
+	stream.avail_in = size;
+	stream.next_out = out = xmalloc(inflated_size);
+	stream.avail_out = inflated_size;
+	inflateInit(&stream);
+	st = inflate(&stream, Z_FINISH);
+	if ((st != Z_STREAM_END) || stream.total_out != inflated_size) {
+		free(out);
+		return NULL;
+	}
+	return out;
+}
+
+static int apply_binary_fragment(struct buffer_desc *desc, struct patch *patch)
+{
+	unsigned long dst_size;
+	struct fragment *fragment = patch->fragments;
+	void *data;
+	void *result;
+
+	data = inflate_it(fragment->patch, fragment->size,
+			  patch->deflate_origlen);
+	if (!data)
+		return error("corrupt patch data");
+	switch (patch->is_binary) {
+	case BINARY_DELTA_DEFLATED:
+		result = patch_delta(desc->buffer, desc->size,
+				     data,
+				     patch->deflate_origlen,
+				     &dst_size);
+		free(desc->buffer);
+		desc->buffer = result;
+		free(data);
+		break;
+	case BINARY_LITERAL_DEFLATED:
+		free(desc->buffer);
+		desc->buffer = data;
+		dst_size = patch->deflate_origlen;
+		break;
+	}
+	if (!desc->buffer)
+		return -1;
+	desc->size = desc->alloc = dst_size;
+	return 0;
+}
+
+static int apply_binary(struct buffer_desc *desc, struct patch *patch)
 {
-	struct fragment *frag = patch->fragments;
 	const char *name = patch->old_name ? patch->old_name : patch->new_name;
+	unsigned char sha1[20];
+	unsigned char hdr[50];
+	int hdrlen;
 
-	if (patch->is_binary) {
-		unsigned char sha1[20];
+	if (!allow_binary_replacement)
+		return error("cannot apply binary patch to '%s' "
+			     "without --allow-binary-replacement",
+			     name);
 
-		if (!allow_binary_replacement)
-			return error("cannot apply binary patch to '%s' "
-				     "without --allow-binary-replacement",
-				     name);
+	/* For safety, we require patch index line to contain
+	 * full 40-byte textual SHA1 for old and new, at least for now.
+	 */
+	if (strlen(patch->old_sha1_prefix) != 40 ||
+	    strlen(patch->new_sha1_prefix) != 40 ||
+	    get_sha1_hex(patch->old_sha1_prefix, sha1) ||
+	    get_sha1_hex(patch->new_sha1_prefix, sha1))
+		return error("cannot apply binary patch to '%s' "
+			     "without full index line", name);
 
-		/* For safety, we require patch index line to contain
-		 * full 40-byte textual SHA1 for old and new, at least for now.
+	if (patch->old_name) {
+		/* See if the old one matches what the patch
+		 * applies to.
 		 */
-		if (strlen(patch->old_sha1_prefix) != 40 ||
-		    strlen(patch->new_sha1_prefix) != 40 ||
-		    get_sha1_hex(patch->old_sha1_prefix, sha1) ||
-		    get_sha1_hex(patch->new_sha1_prefix, sha1))
-			return error("cannot apply binary patch to '%s' "
-				     "without full index line", name);
-
-		if (patch->old_name) {
-			unsigned char hdr[50];
-			int hdrlen;
-
-			/* See if the old one matches what the patch
-			 * applies to.
-			 */
-			write_sha1_file_prepare(desc->buffer, desc->size,
-						blob_type, sha1, hdr, &hdrlen);
-			if (strcmp(sha1_to_hex(sha1), patch->old_sha1_prefix))
-				return error("the patch applies to '%s' (%s), "
-					     "which does not match the "
-					     "current contents.",
-					     name, sha1_to_hex(sha1));
-		}
-		else {
-			/* Otherwise, the old one must be empty. */
-			if (desc->size)
-				return error("the patch applies to an empty "
-					     "'%s' but it is not empty", name);
-		}
+		write_sha1_file_prepare(desc->buffer, desc->size,
+					blob_type, sha1, hdr, &hdrlen);
+		if (strcmp(sha1_to_hex(sha1), patch->old_sha1_prefix))
+			return error("the patch applies to '%s' (%s), "
+				     "which does not match the "
+				     "current contents.",
+				     name, sha1_to_hex(sha1));
+	}
+	else {
+		/* Otherwise, the old one must be empty. */
+		if (desc->size)
+			return error("the patch applies to an empty "
+				     "'%s' but it is not empty", name);
+	}
+
+	get_sha1_hex(patch->new_sha1_prefix, sha1);
+	if (!memcmp(sha1, null_sha1, 20)) {
+		free(desc->buffer);
+		desc->alloc = desc->size = 0;
+		desc->buffer = NULL;
+		return 0; /* deletion patch */
+	}
 
-		/* For now, we do not record post-image data in the patch,
-		 * and require the object already present in the recipient's
-		 * object database.
+	if (has_sha1_file(sha1)) {
+		/* We already have the postimage */
+		char type[10];
+		unsigned long size;
+
+		free(desc->buffer);
+		desc->buffer = read_sha1_file(sha1, type, &size);
+		if (!desc->buffer)
+			return error("the necessary postimage %s for "
+				     "'%s' cannot be read",
+				     patch->new_sha1_prefix, name);
+		desc->alloc = desc->size = size;
+	}
+	else {
+		/* We have verified desc matches the preimage;
+		 * apply the patch data to it, which is stored
+		 * in the patch->fragments->{patch,size}.
 		 */
-		if (desc->buffer) {
-			free(desc->buffer);
-			desc->alloc = desc->size = 0;
-		}
-		get_sha1_hex(patch->new_sha1_prefix, sha1);
-
-		if (memcmp(sha1, null_sha1, 20)) {
-			char type[10];
-			unsigned long size;
-
-			desc->buffer = read_sha1_file(sha1, type, &size);
-			if (!desc->buffer)
-				return error("the necessary postimage %s for "
-					     "'%s' does not exist",
-					     patch->new_sha1_prefix, name);
-			desc->alloc = desc->size = size;
-		}
+		if (apply_binary_fragment(desc, patch))
+			return error("binary patch does not apply to '%s'",
+				     name);
 
-		return 0;
+		/* verify that the result matches */
+		write_sha1_file_prepare(desc->buffer, desc->size, blob_type,
+					sha1, hdr, &hdrlen);
+		if (strcmp(sha1_to_hex(sha1), patch->new_sha1_prefix))
+			return error("binary patch to '%s' creates incorrect result", name);
 	}
 
+	return 0;
+}
+
+static int apply_fragments(struct buffer_desc *desc, struct patch *patch)
+{
+	struct fragment *frag = patch->fragments;
+	const char *name = patch->old_name ? patch->old_name : patch->new_name;
+
+	if (patch->is_binary)
+		return apply_binary(desc, patch);
+
 	while (frag) {
 		if (apply_one_fragment(desc, frag) < 0)
 			return error("patch failed: %s:%ld",
@@ -1983,7 +2158,8 @@ int main(int argc, char **argv)
 			diffstat = 1;
 			continue;
 		}
-		if (!strcmp(arg, "--allow-binary-replacement")) {
+		if (!strcmp(arg, "--allow-binary-replacement") ||
+		    !strcmp(arg, "--binary")) {
 			allow_binary_replacement = 1;
 			continue;
 		}
diff --git a/base85.c b/base85.c
new file mode 100644
index 000000000..a9e97f89d
--- /dev/null
+++ b/base85.c
@@ -0,0 +1,140 @@
+#include "cache.h"
+
+#undef DEBUG_85
+
+#ifdef DEBUG_85
+#define say(a) fprintf(stderr, a)
+#define say1(a,b) fprintf(stderr, a, b)
+#define say2(a,b,c) fprintf(stderr, a, b, c)
+#else
+#define say(a) do {} while(0)
+#define say1(a,b) do {} while(0)
+#define say2(a,b,c) do {} while(0)
+#endif
+
+static const char en85[] = {
+	'0', '1', '2', '3', '4', '5', '6', '7', '8', '9',
+	'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J',
+	'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T',
+	'U', 'V', 'W', 'X', 'Y', 'Z',
+	'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j',
+	'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't',
+	'u', 'v', 'w', 'x', 'y', 'z',
+	'!', '#', '$', '%', '&', '(', ')', '*', '+', '-',
+	';', '<', '=', '>', '?', '@', '^', '_',	'`', '{',
+	'|', '}', '~'
+};
+
+static char de85[256];
+static void prep_base85(void)
+{
+	int i;
+	if (de85['Z'])
+		return;
+	for (i = 0; i < ARRAY_SIZE(en85); i++) {
+		int ch = en85[i];
+		de85[ch] = i + 1;
+	}
+}
+
+int decode_85(char *dst, char *buffer, int len)
+{
+	prep_base85();
+
+	say2("decode 85 <%.*s>", len/4*5, buffer);
+	while (len) {
+		unsigned acc = 0;
+		int de, cnt = 4;
+		unsigned char ch;
+		do {
+			ch = *buffer++;
+			de = de85[ch];
+			if (--de < 0)
+				return error("invalid base85 alphabet %c", ch);
+			acc = acc * 85 + de;
+		} while (--cnt);
+		ch = *buffer++;
+		de = de85[ch];
+		if (--de < 0)
+			return error("invalid base85 alphabet %c", ch);
+		/*
+		 * Detect overflow.  The largest
+		 * 5-letter possible is "|NsC0" to
+		 * encode 0xffffffff, and "|NsC" gives
+		 * 0x03030303 at this point (i.e.
+		 * 0xffffffff = 0x03030303 * 85).
+		 */
+		if (0x03030303 < acc ||
+		    0xffffffff - de < (acc *= 85))
+			error("invalid base85 sequence %.5s", buffer-5);
+		acc += de;
+		say1(" %08x", acc);
+
+		cnt = (len < 4) ? len : 4;
+		len -= cnt;
+		do {
+			acc = (acc << 8) | (acc >> 24);
+			*dst++ = acc;
+		} while (--cnt);
+	}
+	say("\n");
+
+	return 0;
+}
+
+void encode_85(char *buf, unsigned char *data, int bytes)
+{
+	prep_base85();
+
+	say("encode 85");
+	while (bytes) {
+		unsigned acc = 0;
+		int cnt;
+		for (cnt = 24; cnt >= 0; cnt -= 8) {
+			int ch = *data++;
+			acc |= ch << cnt;
+			if (--bytes == 0)
+				break;
+		}
+		say1(" %08x", acc);
+		for (cnt = 4; cnt >= 0; cnt--) {
+			int val = acc % 85;
+			acc /= 85;
+			buf[cnt] = en85[val];
+		}
+		buf += 5;
+	}
+	say("\n");
+
+	*buf = 0;
+}
+
+#ifdef DEBUG_85
+int main(int ac, char **av)
+{
+	char buf[1024];
+
+	if (!strcmp(av[1], "-e")) {
+		int len = strlen(av[2]);
+		encode_85(buf, av[2], len);
+		if (len <= 26) len = len + 'A' - 1;
+		else len = len + 'a' - 26 + 1;
+		printf("encoded: %c%s\n", len, buf);
+		return 0;
+	}
+	if (!strcmp(av[1], "-d")) {
+		int len = *av[2];
+		if ('A' <= len && len <= 'Z') len = len - 'A' + 1;
+		else len = len - 'a' + 26 + 1;
+		decode_85(buf, av[2]+1, len);
+		printf("decoded: %.*s\n", len, buf);
+		return 0;
+	}
+	if (!strcmp(av[1], "-t")) {
+		char t[4] = { -1,-1,-1,-1 };
+		encode_85(buf, t, 4);
+		printf("encoded: D%s\n", buf);
+		return 0;
+	}
+}
+#endif
diff --git a/cache.h b/cache.h
index 9d0ddcff0..4b7a43925 100644
--- a/cache.h
+++ b/cache.h
@@ -363,4 +363,8 @@ extern int receive_keep_pack(int fd[2], const char *me, int quiet);
 /* pager.c */
 extern void setup_pager(void);
 
+/* base85 */
+int decode_85(char *dst, char *line, int linelen);
+void encode_85(char *buf, unsigned char *data, int bytes);
+
 #endif /* CACHE_H */
diff --git a/diff.c b/diff.c
index 531527060..7a7b839e5 100644
--- a/diff.c
+++ b/diff.c
@@ -8,6 +8,7 @@
 #include "quote.h"
 #include "diff.h"
 #include "diffcore.h"
+#include "delta.h"
 #include "xdiff-interface.h"
 
 static int use_size_cache;
@@ -391,6 +392,90 @@ static void show_stats(struct diffstat_t* data)
 			total_files, adds, dels);
 }
 
+static unsigned char *deflate_it(char *data,
+				 unsigned long size,
+				 unsigned long *result_size)
+{
+	int bound;
+	unsigned char *deflated;
+	z_stream stream;
+
+	memset(&stream, 0, sizeof(stream));
+	deflateInit(&stream, Z_BEST_COMPRESSION);
+	bound = deflateBound(&stream, size);
+	deflated = xmalloc(bound);
+	stream.next_out = deflated;
+	stream.avail_out = bound;
+
+	stream.next_in = (unsigned char *)data;
+	stream.avail_in = size;
+	while (deflate(&stream, Z_FINISH) == Z_OK)
+		; /* nothing */
+	deflateEnd(&stream);
+	*result_size = stream.total_out;
+	return deflated;
+}
+
+static void emit_binary_diff(mmfile_t *one, mmfile_t *two)
+{
+	void *cp;
+	void *delta;
+	void *deflated;
+	void *data;
+	unsigned long orig_size;
+	unsigned long delta_size;
+	unsigned long deflate_size;
+	unsigned long data_size;
+
+	printf("GIT binary patch\n");
+	/* We could do deflated delta, or we could do just deflated two,
+	 * whichever is smaller.
+	 */
+	delta = NULL;
+	deflated = deflate_it(two->ptr, two->size, &deflate_size);
+	if (one->size && two->size) {
+		delta = diff_delta(one->ptr, one->size,
+				   two->ptr, two->size,
+				   &delta_size, deflate_size);
+		if (delta) {
+			void *to_free = delta;
+			orig_size = delta_size;
+			delta = deflate_it(delta, delta_size, &delta_size);
+			free(to_free);
+		}
+	}
+
+	if (delta && delta_size < deflate_size) {
+		printf("delta %lu\n", orig_size);
+		free(deflated);
+		data = delta;
+		data_size = delta_size;
+	}
+	else {
+		printf("literal %lu\n", two->size);
+		free(delta);
+		data = deflated;
+		data_size = deflate_size;
+	}
+
+	/* emit data encoded in base85 */
+	cp = data;
+	while (data_size) {
+		int bytes = (52 < data_size) ? 52 : data_size;
+		char line[70];
+		data_size -= bytes;
+		if (bytes <= 26)
+			line[0] = bytes + 'A' - 1;
+		else
+			line[0] = bytes - 26 + 'a' - 1;
+		encode_85(line + 1, cp, bytes);
+		cp += bytes;
+		puts(line);
+	}
+	printf("\n");
+	free(data);
+}
+
 #define FIRST_FEW_BYTES 8000
 static int mmfile_is_binary(mmfile_t *mf)
 {
@@ -407,6 +492,7 @@ static void builtin_diff(const char *name_a,
 			 struct diff_filespec *one,
 			 struct diff_filespec *two,
 			 const char *xfrm_msg,
+			 struct diff_options *o,
 			 int complete_rewrite)
 {
 	mmfile_t mf1, mf2;
@@ -451,8 +537,17 @@ static void builtin_diff(const char *name_a,
 	if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
 		die("unable to read files to diff");
 
-	if (mmfile_is_binary(&mf1) || mmfile_is_binary(&mf2))
-		printf("Binary files %s and %s differ\n", lbl[0], lbl[1]);
+	if (mmfile_is_binary(&mf1) || mmfile_is_binary(&mf2)) {
+		/* Quite common confusing case */
+		if (mf1.size == mf2.size &&
+		    !memcmp(mf1.ptr, mf2.ptr, mf1.size))
+			goto free_ab_and_return;
+		if (o->binary)
+			emit_binary_diff(&mf1, &mf2);
+		else
+			printf("Binary files %s and %s differ\n",
+			       lbl[0], lbl[1]);
+	}
 	else {
 		/* Crazy xdl interfaces.. */
 		const char *diffopts = getenv("GIT_DIFF_OPTS");
@@ -928,6 +1023,7 @@ static void run_diff_cmd(const char *pgm,
 			 struct diff_filespec *one,
 			 struct diff_filespec *two,
 			 const char *xfrm_msg,
+			 struct diff_options *o,
 			 int complete_rewrite)
 {
 	if (pgm) {
@@ -937,7 +1033,7 @@ static void run_diff_cmd(const char *pgm,
 	}
 	if (one && two)
 		builtin_diff(name, other ? other : name,
-			     one, two, xfrm_msg, complete_rewrite);
+			     one, two, xfrm_msg, o, complete_rewrite);
 	else
 		printf("* Unmerged path %s\n", name);
 }
@@ -971,7 +1067,7 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)
 
 	if (DIFF_PAIR_UNMERGED(p)) {
 		/* unmerged */
-		run_diff_cmd(pgm, p->one->path, NULL, NULL, NULL, NULL, 0);
+		run_diff_cmd(pgm, p->one->path, NULL, NULL, NULL, NULL, o, 0);
 		return;
 	}
 
@@ -1041,14 +1137,14 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)
 		 * needs to be split into deletion and creation.
 		 */
 		struct diff_filespec *null = alloc_filespec(two->path);
-		run_diff_cmd(NULL, name, other, one, null, xfrm_msg, 0);
+		run_diff_cmd(NULL, name, other, one, null, xfrm_msg, o, 0);
 		free(null);
 		null = alloc_filespec(one->path);
-		run_diff_cmd(NULL, name, other, null, two, xfrm_msg, 0);
+		run_diff_cmd(NULL, name, other, null, two, xfrm_msg, o, 0);
 		free(null);
 	}
 	else
-		run_diff_cmd(pgm, name, other, one, two, xfrm_msg,
+		run_diff_cmd(pgm, name, other, one, two, xfrm_msg, o,
 			     complete_rewrite);
 
 	free(name_munged);
@@ -1147,6 +1243,10 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		options->rename_limit = strtoul(arg+2, NULL, 10);
 	else if (!strcmp(arg, "--full-index"))
 		options->full_index = 1;
+	else if (!strcmp(arg, "--binary")) {
+		options->output_format = DIFF_FORMAT_PATCH;
+		options->full_index = options->binary = 1;
+	}
 	else if (!strcmp(arg, "--name-only"))
 		options->output_format = DIFF_FORMAT_NAME;
 	else if (!strcmp(arg, "--name-status"))
diff --git a/diff.h b/diff.h
index b3b2c4dd2..d05260840 100644
--- a/diff.h
+++ b/diff.h
@@ -28,6 +28,7 @@ struct diff_options {
 		 with_raw:1,
 		 with_stat:1,
 		 tree_in_recursive:1,
+		 binary:1,
 		 full_index:1,
 		 silent_on_remove:1,
 		 find_copies_harder:1;
diff --git a/t/t4012-diff-binary.sh b/t/t4012-diff-binary.sh
new file mode 100755
index 000000000..bdd95c0d3
--- /dev/null
+++ b/t/t4012-diff-binary.sh
@@ -0,0 +1,85 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Junio C Hamano
+#
+
+test_description='Binary diff and apply
+'
+
+. ./test-lib.sh
+
+test_expect_success 'prepare repository' \
+	'echo AIT >a && echo BIT >b && echo CIT >c && echo DIT >d &&
+	 git-update-index --add a b c d &&
+	 echo git >a &&
+	 cat ../test4012.png >b &&
+	 echo git >c &&
+	 cat b b >d'
+
+test_expect_success 'diff without --binary' \
+	'git-diff | git-apply --stat --summary >current &&
+	 cmp current - <<\EOF
+ a |    2 +-
+ b |  Bin
+ c |    2 +-
+ d |  Bin
+ 4 files changed, 2 insertions(+), 2 deletions(-)
+EOF'
+
+test_expect_success 'diff with --binary' \
+	'git-diff --binary | git-apply --stat --summary >current &&
+	 cmp current - <<\EOF
+ a |    2 +-
+ b |  Bin
+ c |    2 +-
+ d |  Bin
+ 4 files changed, 2 insertions(+), 2 deletions(-)
+EOF'
+
+# apply needs to be able to skip the binary material correctly
+# in order to report the line number of a corrupt patch.
+test_expect_success 'apply detecting corrupt patch correctly' \
+	'git-diff | sed -e 's/-CIT/xCIT/' >broken &&
+	 if git-apply --stat --summary broken 2>detected
+	 then
+		echo unhappy - should have detected an error
+		(exit 1)
+	 else
+		echo happy
+	 fi &&
+	 detected=`cat detected` &&
+	 detected=`expr "$detected" : "fatal.*at line \\([0-9]*\\)\$"` &&
+	 detected=`sed -ne "${detected}p" broken` &&
+	 test "$detected" = xCIT'
+
+test_expect_success 'apply detecting corrupt patch correctly' \
+	'git-diff --binary | sed -e 's/-CIT/xCIT/' >broken &&
+	 if git-apply --stat --summary broken 2>detected
+	 then
+		echo unhappy - should have detected an error
+		(exit 1)
+	 else
+		echo happy
+	 fi &&
+	 detected=`cat detected` &&
+	 detected=`expr "$detected" : "fatal.*at line \\([0-9]*\\)\$"` &&
+	 detected=`sed -ne "${detected}p" broken` &&
+	 test "$detected" = xCIT'
+
+test_expect_success 'initial commit' 'git-commit -a -m initial'
+
+# Try removal (b), modification (d), and creation (e).
+test_expect_success 'diff-index with --binary' \
+	'echo AIT >a && mv b e && echo CIT >c && cat e >d &&
+	 git-update-index --add --remove a b c d e &&
+	 tree0=`git-write-tree` &&
+	 git-diff --cached --binary >current &&
+	 git-apply --stat --summary current'
+
+test_expect_success 'apply binary patch' \
+	'git-reset --hard &&
+	 git-apply --binary --index <current &&
+	 tree1=`git-write-tree` &&
+	 test "$tree1" = "$tree0"'
+
+test_done
diff --git a/t/test4012.png b/t/test4012.png
new file mode 100644
index 000000000..7b181d15c
--- /dev/null
+++ b/t/test4012.png