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

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

[Marco Costalba]

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(-)

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

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

[Nicolas Pitre]

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

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

[Marco Costalba]

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 ?

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

Thanks
Marco

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

[Junio C Hamano]

"Marco Costalba" <mcostalba@gmail.com> writes:

> 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(-)

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.