Re: [PATCH v2 08/19] reftable: reading/writing blocks

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Thu, Sep 09 2021, Han-Wen Nienhuys via GitGitGadget wrote:

> From: Han-Wen Nienhuys <hanwen@google.com>
>
> The reftable format is structured as a sequence of block. Within a block,
> records are prefix compressed, with an index of offsets for fully expand keys to
> enable binary search within blocks.
> [...]
> +	if (typ == BLOCK_TYPE_LOG) {
> +		int block_header_skip = 4 + header_off;
> +		uLongf dst_len = sz - block_header_skip; /* total size of dest
> +							    buffer. */
> +		uLongf src_len = block->len - block_header_skip;
> +		/* Log blocks specify the *uncompressed* size in their header.
> +		 */
> +		uint8_t *uncompressed = reftable_malloc(sz);
> +
> +		/* Copy over the block header verbatim. It's not compressed. */
> +		memcpy(uncompressed, block->data, block_header_skip);
> +
> +		/* Uncompress */
> +		if (Z_OK !=
> +		    uncompress2(uncompressed + block_header_skip, &dst_len,
> +				block->data + block_header_skip, &src_len)) {
> +			reftable_free(uncompressed);
> +			return REFTABLE_ZLIB_ERROR;
> +		}
> +
> +		if (dst_len + block_header_skip != sz)
> +			return REFTABLE_FORMAT_ERROR;
> +
> +		/* We're done with the input data. */
> +		reftable_block_done(block);
> +		block->data = uncompressed;
> +		block->len = sz;
> +		block->source = malloc_block_source();
> +		full_block_size = src_len + block_header_skip;

I haven't tried to re-implement this, but in 07/19 we're adding an
uncompress2() fallaback function, and here's the only place where it'll
get used.

Looking at the uncompress2() implementation isn't this a rather trivial
wrapper for something we'd get from git.git's zlib.c, i.e. you seem to
just need the "avail_in" here, which we're checking in various other
parts of the codebase.

I suspect that the reason for this is that this code is trying to keep
at arms length from the rest of git.git, but if so some ifdef here if
it's compiling in git.git seems better than having a compat interface in
git.git just for doing something its zlib.c API provides.

Anyway, re Junio's ask if this is ready for "next" in the latest What's
Cooking I don't think this should be a blocker, just wondering if
anyone's interested in some post-cleanup at some point, and whether I'm
missing something about uncompress2() v.s. zlib.c.