Nguyễn Thái Ngọc Duy wrote:
> Most of deflation code is simply "given this buffer, just deflate
> it". Make a common routine and reuse it instead.
I like this idea. But:
> There is possibly a regression here.
Right.
> --- a/archive-zip.c
> +++ b/archive-zip.c
Looks good.
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -130,28 +130,10 @@ 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);
> + void *out = git_deflate(*pptr, &size, pack_compression_level);
> + free(*pptr);
> *pptr = out;
On error, previously *pptr and size would reflect a truncated result,
but now *pptr is NULL and size is 0. Both results are silly.
It would be nicer if the caller (or do_compress itself) could check
for errors and report them.
> --- a/diff.c
> +++ b/diff.c
> @@ -1713,7 +1689,8 @@ static void emit_binary_diff_body(FILE *file, mmfile_t *one, mmfile_t *two, char
> * 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);
[...]
> @@ -1721,7 +1698,7 @@ 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);
[...]
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -1025,24 +1025,13 @@ static int store_object(
[...]
> - 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);
[...]
> @@ -1053,15 +1042,10 @@ static int store_object(
[...]
> - 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);
Likewise.
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -420,33 +420,10 @@ static int post_rpc(struct rpc_state *rpc)
> * we can try to deflate it ourselves, this may save on.
> * the transfer time.
> */
> - size_t size;
> - z_stream stream;
> - int ret;
> -
> - memset(&stream, 0, sizeof(stream));
> - ret = deflateInit2(&stream, Z_BEST_COMPRESSION,
> - Z_DEFLATED, (15 + 16),
As Shawn mentioned, this requests gzip encoding with the default
window size.
> - ret = deflate(&stream, Z_FINISH);
> - if (ret != Z_STREAM_END)
> - die("cannot deflate request; zlib deflate error %d", ret);
> -
> - ret = deflateEnd(&stream);
> - if (ret != Z_OK)
> - die("cannot deflate request; zlib end error %d", ret);
> -
> - size = stream.total_out;
> + unsigned long size = rpc->len;
> + gzip_body = git_deflate(rpc->buf, &size, Z_BEST_COMPRESSION);
> + if (!gzip_body)
> + die("cannot deflate request; zlib deflate error");
The zlib error codes are very helpful for debugging, so I would be sad
to see them go.
Thanks,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html