[PATCH] Reduce zlib deflate code duplication

From

Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

Date

Aug 30, 2010, 01:46 UTC

Message-ID

<1283132815-3277-1-git-send-email-pclouds@gmail.com>

In-Reply-To

<1282856164-5126-1-git-send-email-pclouds@gmail.com>

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 remote-curl is now excluded from the patch.
 
 git_deflate() returns zlib error code and will be used to print
 something out. I'm not sure if it should die() inside diff.c.
 It error() onlys for now.

 archive-zip.c          |   37 ++-----------------------------------
 builtin/pack-objects.c |   27 ++++++---------------------
 cache.h                |    1 +
 diff.c                 |   41 +++++++++++++++--------------------------
 fast-import.c          |   45 +++++++++++++++++----------------------------
 wrapper.c              |   31 +++++++++++++++++++++++++++++++
 6 files changed, 72 insertions(+), 110 deletions(-)

diff --git a/archive-zip.c b/archive-zip.c
index cf28504..ed176ca 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -87,39 +87,6 @@ static void copy_le32(unsigned char *dest, unsigned int n)
 	dest[3] = 0xff & (n >> 030);
 }
 
-static void *zlib_deflate(void *data, unsigned long size,
-		int compression_level, unsigned long *compressed_size)
-{
-	z_stream stream;
-	unsigned long maxsize;
-	void *buffer;
-	int result;
-
-	memset(&stream, 0, sizeof(stream));
-	deflateInit(&stream, compression_level);
-	maxsize = deflateBound(&stream, size);
-	buffer = xmalloc(maxsize);
-
-	stream.next_in = data;
-	stream.avail_in = size;
-	stream.next_out = buffer;
-	stream.avail_out = maxsize;
-
-	do {
-		result = deflate(&stream, Z_FINISH);
-	} while (result == Z_OK);
-
-	if (result != Z_STREAM_END) {
-		free(buffer);
-		return NULL;
-	}
-
-	deflateEnd(&stream);
-	*compressed_size = stream.total_out;
-
-	return buffer;
-}
-
 static int write_zip_entry(struct archiver_args *args,
 		const unsigned char *sha1, const char *path, size_t pathlen,
 		unsigned int mode, void *buffer, unsigned long size)
@@ -164,8 +131,8 @@ static int write_zip_entry(struct archiver_args *args,
 	}
 
 	if (method == 8) {
-		deflated = zlib_deflate(buffer, size, args->compression_level,
-				&compressed_size);
+		compressed_size = size;
+		deflated = git_deflate(buffer, &compressed_size, args->compression_level, NULL);
 		if (deflated && compressed_size - 6 < size) {
 			/* ZLIB --> raw compressed data (see RFC 1950) */
 			/* CMF and FLG ... */
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 0e81673..9e78e88 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -130,28 +130,13 @@ static void *get_delta(struct object_entry *entry)
 
 static unsigned long do_compress(void **pptr, unsigned long size)
 {
-	z_stream stream;
-	void *in, *out;
-	unsigned long maxsize;
-
-	memset(&stream, 0, sizeof(stream));
-	deflateInit(&stream, pack_compression_level);
-	maxsize = deflateBound(&stream, size);
-
-	in = *pptr;
-	out = xmalloc(maxsize);
+	int zlib_error;
+	void *out = git_deflate(*pptr, &size, pack_compression_level, &zlib_error);
+	if (!out)
+		die("failed to compress, zlib error %d", zlib_error);
+	free(*pptr);
 	*pptr = out;
-
-	stream.next_in = in;
-	stream.avail_in = size;
-	stream.next_out = out;
-	stream.avail_out = maxsize;
-	while (deflate(&stream, Z_FINISH) == Z_OK)
-		; /* nothing */
-	deflateEnd(&stream);
-
-	free(in);
-	return stream.total_out;
+	return size;
 }
 
 /*
diff --git a/cache.h b/cache.h
index eb77e1d..afca130 100644
--- a/cache.h
+++ b/cache.h
@@ -22,6 +22,7 @@
 void git_inflate_init(z_streamp strm);
 void git_inflate_end(z_streamp strm);
 int git_inflate(z_streamp strm, int flush);
+void *git_deflate(void *buf, unsigned long *size, int compression_level, int *error);
 
 #if defined(DT_UNKNOWN) && !defined(NO_D_TYPE_IN_DIRENT)
 #define DTYPE(de)	((de)->d_type)
diff --git a/diff.c b/diff.c
index 6fb97d4..56ef7a0 100644
--- a/diff.c
+++ b/diff.c
@@ -1674,30 +1674,6 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len)
 	}
 }
 
-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, zlib_compression_level);
-	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_body(FILE *file, mmfile_t *one, mmfile_t *two, char *prefix)
 {
 	void *cp;
@@ -1708,12 +1684,19 @@ static void emit_binary_diff_body(FILE *file, mmfile_t *one, mmfile_t *two, char
 	unsigned long delta_size;
 	unsigned long deflate_size;
 	unsigned long data_size;
+	int zlib_error;
 
 	/* 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);
+	deflate_size = two->size;
+	deflated = git_deflate(two->ptr, &deflate_size, zlib_compression_level, &zlib_error);
+	if (!deflated) {
+		error("failed to compress for binary diff prefix %s, zlib error %d",
+		      prefix, zlib_error);
+		return;
+	}
 	if (one->size && two->size) {
 		delta = diff_delta(one->ptr, one->size,
 				   two->ptr, two->size,
@@ -1721,8 +1704,14 @@ static void emit_binary_diff_body(FILE *file, mmfile_t *one, mmfile_t *two, char
 		if (delta) {
 			void *to_free = delta;
 			orig_size = delta_size;
-			delta = deflate_it(delta, delta_size, &delta_size);
+			delta = git_deflate(delta, &delta_size, zlib_compression_level, NULL);
 			free(to_free);
+			if (!delta) {
+				free(deflated);
+				error("failed to compress for delta prefix %s, zlib error %d",
+				      prefix, zlib_error);
+				return;
+			}
 		}
 	}
 
diff --git a/fast-import.c b/fast-import.c
index dd51ac4..cbdab25 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -993,7 +993,8 @@ static int store_object(
 	unsigned char sha1[20];
 	unsigned long hdrlen, deltalen;
 	git_SHA_CTX c;
-	z_stream s;
+	unsigned long compressed_size;
+	int zlib_error;
 
 	hdrlen = sprintf((char *)hdr,"%s %lu", typename(type),
 		(unsigned long)dat->len) + 1;
@@ -1025,24 +1026,15 @@ static int store_object(
 	} else
 		delta = NULL;
 
-	memset(&s, 0, sizeof(s));
-	deflateInit(&s, pack_compression_level);
-	if (delta) {
-		s.next_in = delta;
-		s.avail_in = deltalen;
-	} else {
-		s.next_in = (void *)dat->buf;
-		s.avail_in = dat->len;
-	}
-	s.avail_out = deflateBound(&s, s.avail_in);
-	s.next_out = out = xmalloc(s.avail_out);
-	while (deflate(&s, Z_FINISH) == Z_OK)
-		/* nothing */;
-	deflateEnd(&s);
+	compressed_size = delta ? deltalen : dat->len;
+	out = git_deflate(delta ? delta : dat->buf, &compressed_size,
+			  pack_compression_level, &zlib_error);
+	if (!out)
+		die("failed to compress, zlib error %d", zlib_error);
 
 	/* Determine if we should auto-checkpoint. */
-	if ((max_packsize && (pack_size + 60 + s.total_out) > max_packsize)
-		|| (pack_size + 60 + s.total_out) < pack_size) {
+	if ((max_packsize && (pack_size + 60 + compressed_size) > max_packsize)
+		|| (pack_size + 60 + compressed_size) < pack_size) {
 
 		/* This new object needs to *not* have the current pack_id. */
 		e->pack_id = pack_id + 1;
@@ -1053,15 +1045,12 @@ static int store_object(
 			free(delta);
 			delta = NULL;
 
-			memset(&s, 0, sizeof(s));
-			deflateInit(&s, pack_compression_level);
-			s.next_in = (void *)dat->buf;
-			s.avail_in = dat->len;
-			s.avail_out = deflateBound(&s, s.avail_in);
-			s.next_out = out = xrealloc(out, s.avail_out);
-			while (deflate(&s, Z_FINISH) == Z_OK)
-				/* nothing */;
-			deflateEnd(&s);
+			free(out);
+			compressed_size = dat->len;
+			out = git_deflate(dat->buf, &compressed_size,
+					  pack_compression_level, &zlib_error);
+			if (!out)
+				die("failed to compress, zlib error %d", zlib_error);
 		}
 	}
 
@@ -1096,8 +1085,8 @@ static int store_object(
 		pack_size += hdrlen;
 	}
 
-	sha1write(pack_file, out, s.total_out);
-	pack_size += s.total_out;
+	sha1write(pack_file, out, compressed_size);
+	pack_size += compressed_size;
 
 	e->idx.crc32 = crc32_end(pack_file);
 
diff --git a/wrapper.c b/wrapper.c
index fd8ead3..d0d3de9 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -289,6 +289,37 @@ int git_inflate(z_streamp strm, int flush)
 	return ret;
 }
 
+void *git_deflate(void *in, unsigned long *size, int compression_level, int *error)
+{
+	z_stream stream;
+	void *out;
+	unsigned long maxsize;
+	int result;
+
+	memset(&stream, 0, sizeof(stream));
+	deflateInit(&stream, compression_level);
+	maxsize = deflateBound(&stream, *size);
+
+	out = xmalloc(maxsize);
+	stream.next_in = in;
+	stream.avail_in = *size;
+	stream.next_out = out;
+	stream.avail_out = maxsize;
+	while ((result = deflate(&stream, Z_FINISH)) == Z_OK)
+		; /* nothing */
+
+	if (result != Z_STREAM_END ||
+	    (result = deflateEnd(&stream)) != Z_OK) {
+		if (error)
+			*error = result;
+		*size = 0;
+		free(out);
+		return NULL;
+	}
+	*size = stream.total_out;
+	return out;
+}
+
 int odb_mkstemp(char *template, size_t limit, const char *pattern)
 {
 	int fd;

-- 
1.7.1.rc1.69.g24c2f7