[systemd-devel] [PATCH 1/2] journal: add LZ4 as optional compressor

[Lennart Poettering]

On Sat, 05.07.14 20:56, Zbigniew Jędrzejewski-Szmek (zbyszek at in.waw.pl) wrote:

> -bool uncompress_blob(const void *src, uint64_t src_size,
> -                     void **dst, uint64_t *dst_alloc_size, uint64_t* dst_size, uint64_t dst_max) {
> +int compress_blob_lz4(const void *src, uint64_t src_size, void *dst, uint64_t *dst_size) {
> +#ifdef HAVE_LZ4
> +        int r;
>  
> +        assert(src);
> +        assert(src_size > 0);
> +        assert(dst);
> +        assert(dst_size);
> +
> +        /* Returns false if we couldn't compress the data or the
> +         * compressed result is longer than the original */
> +
> +        if (src_size < 9)
> +                return -ENOSPC;
> +
> +        r = LZ4_compress_limitedOutput(src, dst + 8, src_size, src_size - 8 - 1);
> +        if (r <= 0)
> +                return -ENOSPC;

Maybe EAGAIN? Would feel a bit like a kernel API then ;-).

> +        buf1 = malloc(LZ4_BUFSIZE);
> +        buf2 = malloc(LZ4_BUFSIZE);
> +        out = malloc(LZ4_COMPRESSBOUND(LZ4_BUFSIZE));
> +        if (!buf1 || !buf2 || !out)
> +                return log_oom();

Shouldn't we just return -ENOMEM here? Feels like a "library" function, not
a "program" function, hence it should not log at the anything but debug, really....

> +        out = malloc(4*LZ4_BUFSIZE);
> +        if (!out)
> +                return log_oom();

Same here...

Looks good otheriwse.

BTW, have you checked whether reuseing the XZ context might make the XZ
more competitive?

Lennart

-- 
Lennart Poettering, Red Hat