threads / patch / 11572

patch, 6 partsUse new compress helpers in builtin-pack-objects.c

Subject: [PATCH 6/6] Use new compress helpers in builtin-pack-objects.c

## tl;dr

4 messages between Jan 10, 2008 and Jan 11, 2008. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Marco Costalba· Jan 10, 2008, 21:34 UTC · lore
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
---
It's 'compress helper' not 'decomrpess helper' of course.
All my patch series is misnamed by my idiocy!

This is the last one for this evening, this is interesting because pack_compression_level instead of the usual zlib_compression_level is used.

The goal of this patch series is to have some comment before to procede with remaining conversions.

When compression is fully abstarceted out I will go with decompression (inflate).

At the end of this job perhaps we have something that allows us to have fun with different compression libraries.

In any case IMHO this patch series is valid 'per se' as a cleanup and (a lot of) code lines removing.

 builtin-pack-objects.c |   21 ++++-----------------
 1 files changed, 4 insertions(+), 17 deletions(-)
Show changes to builtin-pack-objects.c +4 −17
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index a39cb82..682d7fb 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1,5 +1,6 @@
 #include "builtin.h"
 #include "cache.h"
+#include "compress.h"
 #include "attr.h"
 #include "object.h"
 #include "blob.h"
@@ -409,9 +410,7 @@ static unsigned long write_object(struct sha1file *f,
 				 */

 	if (!to_reuse) {
-		z_stream stream;
-		unsigned long maxsize;
-		void *out;
+		unsigned char *out = NULL;
 		if (!usable_delta) {
 			buf = read_sha1_file(entry->idx.sha1, &obj_type, &size);
 			if (!buf)
@@ -432,20 +431,8 @@ static unsigned long write_object(struct sha1file *f,
 				OBJ_OFS_DELTA : OBJ_REF_DELTA;
 		}
 		/* compress the data to store and put compressed length in datalen */
-		memset(&stream, 0, sizeof(stream));
-		deflateInit(&stream, pack_compression_level);
-		maxsize = deflateBound(&stream, size);
-		out = xmalloc(maxsize);
-		/* Compress it */
-		stream.next_in = buf;
-		stream.avail_in = size;
-		stream.next_out = out;
-		stream.avail_out = maxsize;
-		while (deflate(&stream, Z_FINISH) == Z_OK)
-			/* nothing */;
-		deflateEnd(&stream);
-		datalen = stream.total_out;
-		deflateEnd(&stream);
+		datalen = z_deflate_all(pack_compression_level, buf, size, &out);
+
 		/*
 		 * The object header is a byte of 'type' followed by zero or
 		 * more bytes of length.

-- 
1.5.4.rc2.89.g1b3f-dirty
Nicolas Pitre· Jan 10, 2008, 22:26 UTC · re: Marco Costalba · lore

Re: [PATCH 6/6] Use new compress helpers in builtin-pack-objects.c

On Thu, 10 Jan 2008, Marco Costalba wrote:
> This is the last one for this evening, this is interesting
> because pack_compression_level instead of the usual
> zlib_compression_level is used.

See documentation for the core.compression, core.loosecompression and pack.compression config variables.

Nicolas
Marco Costalba· Jan 11, 2008, 06:10 UTC · re: Marco Costalba · lore

Re: [PATCH 6/6] Use new compress helpers in builtin-pack-objects.c

On Jan 10, 2008 10:34 PM, Marco Costalba <mcostalba@gmail.com> wrote:
Show 6 quoted lines
>
> -               while (deflate(&stream, Z_FINISH) == Z_OK)
> -                       /* nothing */;
> -               deflateEnd(&stream);
> -               datalen = stream.total_out;
> -               deflateEnd(&stream);

One thing I would like someone to clarify is why deflateEnd(&stream) is called two times ?

With my patch is called only once as I have seen in all the others places.

Thanks Marco

Junio C Hamano· Jan 11, 2008, 07:26 UTC · re: Marco Costalba · lore

Re: [PATCH 6/6] Use new compress helpers in builtin-pack-objects.c

"Marco Costalba" <mcostalba@gmail.com> writes:
Show 11 quoted lines
> On Jan 10, 2008 10:34 PM, Marco Costalba <mcostalba@gmail.com> wrote:
>>
>> -               while (deflate(&stream, Z_FINISH) == Z_OK)
>> -                       /* nothing */;
>> -               deflateEnd(&stream);
>> -               datalen = stream.total_out;
>> -               deflateEnd(&stream);
>
>
> One thing I would like someone to clarify is why deflateEnd(&stream)
> is called two times ?
I'd call it an insufficient reviewing while accepting
    http://thread.gmane.org/gmane.comp.version-control.git/45996

In fact, the second deflateEnd() has always been returning Z_STREAM_ERROR. We just never checked the error return from that particular deflateEnd().

The first one returns 0 for success. We might want to tighten the check even more to check that.

-- >8 -- pack-objects: remove redundant call to deflateEnd()

We somehow called deflateEnd() on a stream that we have called deflateEnd() on already.

Noticed by Marco.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-pack-objects.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Show changes to builtin-pack-objects.c +1 −1
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index a39cb82..ec10238 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -445,7 +445,7 @@ static unsigned long write_object(struct sha1file *f,
 			/* nothing */;
 		deflateEnd(&stream);
 		datalen = stream.total_out;
-		deflateEnd(&stream);
+
 		/*
 		 * The object header is a byte of 'type' followed by zero or
 		 * more bytes of length.

← back to recent threads