# [PATCH 6/6] Use new compress helpers in builtin-pack-objects.c 4 messages from 2008-01-10 to 2008-01-11. Participants: Marco Costalba, Nicolas Pitre, Junio C Hamano. Thread: https://gitlist.dev/t/11572 ## Marco Costalba, 2008-01-10 21:34 Subject: [PATCH 6/6] Use new compress helpers in builtin-pack-objects.c Message-ID: URL: https://gitlist.dev/e/e5bfff550801101334p5df5adaerf0eeae02ddf28334%40mail.gmail.com ``` Signed-off-by: Marco Costalba --- 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 ``` ## Nicolas Pitre, 2008-01-10 22:26 Subject: Re: [PATCH 6/6] Use new compress helpers in builtin-pack-objects.c Message-ID: URL: https://gitlist.dev/e/alpine.LFD.1.00.0801101724250.3054%40xanadu.home In-Reply-To: ``` 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, 2008-01-11 06:10 Subject: Re: [PATCH 6/6] Use new compress helpers in builtin-pack-objects.c Message-ID: URL: https://gitlist.dev/e/e5bfff550801102210g181a091s28725e24922b9b12%40mail.gmail.com In-Reply-To: ``` On Jan 10, 2008 10:34 PM, Marco Costalba 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 ``` ## Junio C Hamano, 2008-01-11 07:26 Subject: Re: [PATCH 6/6] Use new compress helpers in builtin-pack-objects.c Message-ID: <7vir204ycu.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vir204ycu.fsf%40gitster.siamese.dyndns.org In-Reply-To: ``` "Marco Costalba" writes: > On Jan 10, 2008 10:34 PM, Marco Costalba 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 --- 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. ```