Re: [PATCH 01/11] Introduce stream compress helpers
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 3, 2008, 22:53 UTC
- Message-ID
- <7v8x21slck.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <1201952156-6764-1-git-send-email-mcostalba@gmail.com>
Marco Costalba <mcostalba@gmail.com> writes:
Show 9 quoted lines
> When decompressing a zlib stream use this > helpers instead of calling low level zlib > function. > > This patch introduces the necessary framework, > still no code change. > > This is the first step in generalizing compress and > decompress functions avoiding zlib directly calls.
(Cosmetic)
Your log message lines are wrapped a bit too short, while some of the patch lines are too long.
Show 15 quoted lines
> diff --git a/compress.c b/compress.c
> new file mode 100644
> index 0000000..f6986c3
> --- /dev/null
> +++ b/compress.c
> @@ -0,0 +1,58 @@
> +#include "cache.h"
> +#include "compress.h"
> +
> +unsigned long compress_alloc(z_stream *stream, int level, unsigned long size)
> +{
> + memset(stream, 0, sizeof(*stream));
> + deflateInit(stream, level);
> + return deflateBound(stream, size);
> +}(Naming)
This is not about "allocation", but about "setup".
Show 10 quoted lines
> +int compress_start(z_stream *stream,
> + unsigned char *in, unsigned long in_size,
> + unsigned char *out, unsigned long out_size)
> +{
> + stream->next_out = (out ? out : xmalloc(out_size));
> + stream->avail_out = out_size;
> + stream->next_in = in;
> + stream->avail_in = in_size;
> + return Z_OK;
> +}This returns Z_OK unconditionally and most callers do not even bother checking the return value. Shouldn't this be of type void?
Especially the use of this in if() conditional, after [06/11] changes its use in compress_all(), looks quite ugly.
Show 5 quoted lines
> +unsigned long compress_free(z_stream *stream)
> +{
> + deflateEnd(stream);
> + return stream->total_out;
> +}Eventually, this should check errors from deflateEnd() and propagate that to the caller.