{"thread":{"id":"11572","subject":"[PATCH 6/6] Use new compress helpers in builtin-pack-objects.c","startedAt":"2008-01-10T21:34:49Z","lastAt":"2008-01-11T07:26:09Z","messageCount":4,"participants":["Marco Costalba","Nicolas Pitre","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"64955","messageId":"e5bfff550801101334p5df5adaerf0eeae02ddf28334@mail.gmail.com","threadId":"11572","inReplyTo":null,"subject":"[PATCH 6/6] Use new compress helpers in builtin-pack-objects.c","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2008-01-10T21:34:49Z","receivedAt":"2008-01-10T21:34:49Z","isPatch":true,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"Signed-off-by: Marco Costalba \n---\n\n\nIt's 'compress helper' not 'decomrpess helper' of course.\n\nAll my patch series is misnamed by my idiocy!\n\nThis is the last one for this evening, this is interesting\nbecause pack_compression_level instead of the usual\nzlib_compression_level is used.\n\nThe goal of this patch series is to have some\ncomment before to procede with remaining conversions.\n\nWhen compression is fully abstarceted out I will go with\ndecompression (inflate).\n\nAt the end of this job perhaps we have something that\nallows us to have fun with different compression libraries.\n\nIn any case IMHO this patch series is valid 'per se' as a\ncleanup and (a lot of) code lines removing.\n\n\n builtin-pack-objects.c | 21 ++++-----------------\n 1 files changed, 4 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex a39cb82..682d7fb 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -1,5 +1,6 @@\n #include \"builtin.h\"\n #include \"cache.h\"\n+#include \"compress.h\"\n #include \"attr.h\"\n #include \"object.h\"\n #include \"blob.h\"\n@@ -409,9 +410,7 @@ static unsigned long write_object(struct sha1file *f,\n \t\t\t\t */\n\n \tif (!to_reuse) {\n-\t\tz_stream stream;\n-\t\tunsigned long maxsize;\n-\t\tvoid *out;\n+\t\tunsigned char *out = NULL;\n \t\tif (!usable_delta) {\n \t\t\tbuf = read_sha1_file(entry->idx.sha1, &obj_type, &size);\n \t\t\tif (!buf)\n@@ -432,20 +431,8 @@ static unsigned long write_object(struct sha1file *f,\n \t\t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n \t\t}\n \t\t/* compress the data to store and put compressed length in datalen */\n-\t\tmemset(&stream, 0, sizeof(stream));\n-\t\tdeflateInit(&stream, pack_compression_level);\n-\t\tmaxsize = deflateBound(&stream, size);\n-\t\tout = xmalloc(maxsize);\n-\t\t/* Compress it */\n-\t\tstream.next_in = buf;\n-\t\tstream.avail_in = size;\n-\t\tstream.next_out = out;\n-\t\tstream.avail_out = maxsize;\n-\t\twhile (deflate(&stream, Z_FINISH) == Z_OK)\n-\t\t\t/* nothing */;\n-\t\tdeflateEnd(&stream);\n-\t\tdatalen = stream.total_out;\n-\t\tdeflateEnd(&stream);\n+\t\tdatalen = z_deflate_all(pack_compression_level, buf, size, &out);\n+\n \t\t/*\n \t\t * The object header is a byte of 'type' followed by zero or\n \t\t * more bytes of length.\n-- \n1.5.4.rc2.89.g1b3f-dirty\n"},{"id":"64964","messageId":"alpine.LFD.1.00.0801101724250.3054@xanadu.home","threadId":"11572","inReplyTo":"e5bfff550801101334p5df5adaerf0eeae02ddf28334@mail.gmail.com","subject":"Re: [PATCH 6/6] Use new compress helpers in builtin-pack-objects.c","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-01-10T22:26:09Z","receivedAt":"2008-01-10T22:26:09Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Thu, 10 Jan 2008, Marco Costalba wrote:\n\n> This is the last one for this evening, this is interesting\n> because pack_compression_level instead of the usual\n> zlib_compression_level is used.\n\nSee documentation for the core.compression, core.loosecompression and \npack.compression config variables.\n\n\nNicolas\n"},{"id":"64981","messageId":"e5bfff550801102210g181a091s28725e24922b9b12@mail.gmail.com","threadId":"11572","inReplyTo":"e5bfff550801101334p5df5adaerf0eeae02ddf28334@mail.gmail.com","subject":"Re: [PATCH 6/6] Use new compress helpers in builtin-pack-objects.c","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2008-01-11T06:10:49Z","receivedAt":"2008-01-11T06:10:49Z","isPatch":true,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"On Jan 10, 2008 10:34 PM, Marco Costalba wrote:\n>\n> - while (deflate(&stream, Z_FINISH) == Z_OK)\n> - /* nothing */;\n> - deflateEnd(&stream);\n> - datalen = stream.total_out;\n> - deflateEnd(&stream);\n\n\nOne thing I would like someone to clarify is why deflateEnd(&stream)\nis called two times ?\n\nWith my patch is called only once as I have seen in all the others places.\n\nThanks\nMarco\n"},{"id":"64987","messageId":"7vir204ycu.fsf@gitster.siamese.dyndns.org","threadId":"11572","inReplyTo":"e5bfff550801102210g181a091s28725e24922b9b12@mail.gmail.com","subject":"Re: [PATCH 6/6] Use new compress helpers in builtin-pack-objects.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-11T07:26:09Z","receivedAt":"2008-01-11T07:26:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Marco Costalba\" writes:\n\n> On Jan 10, 2008 10:34 PM, Marco Costalba wrote:\n>>\n>> - while (deflate(&stream, Z_FINISH) == Z_OK)\n>> - /* nothing */;\n>> - deflateEnd(&stream);\n>> - datalen = stream.total_out;\n>> - deflateEnd(&stream);\n>\n>\n> One thing I would like someone to clarify is why deflateEnd(&stream)\n> is called two times ?\n\nI'd call it an insufficient reviewing while accepting\n\n http://thread.gmane.org/gmane.comp.version-control.git/45996\n\nIn fact, the second deflateEnd() has always been returning\nZ_STREAM_ERROR. We just never checked the error return from\nthat particular deflateEnd().\n\nThe first one returns 0 for success. We might want to tighten\nthe check even more to check that.\n\n-- >8 --\npack-objects: remove redundant call to deflateEnd()\n\nWe somehow called deflateEnd() on a stream that we have called\ndeflateEnd() on already.\n\nNoticed by Marco.\n\nSigned-off-by: Junio C Hamano \n---\n builtin-pack-objects.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex a39cb82..ec10238 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -445,7 +445,7 @@ static unsigned long write_object(struct sha1file *f,\n \t\t\t/* nothing */;\n \t\tdeflateEnd(&stream);\n \t\tdatalen = stream.total_out;\n-\t\tdeflateEnd(&stream);\n+\n \t\t/*\n \t\t * The object header is a byte of 'type' followed by zero or\n \t\t * more bytes of length.\n"}]}