Re: [PATCH] LZ4: compression/decompression signedness mismatch

From: Sergey Senozhatsky
Date: Thu Jul 18 2013 - 17:18:50 EST


Hello,

On (07/12/13 12:48), Sergey Senozhatsky wrote:
> On (07/12/13 11:28), Yann Collet wrote:
> > The reference implementation, hosted at :ï
> > [1]https://code.google.com/p/lz4/
> > only proposes char* (signed) types as part of the interface contract.
> > I would recommend to keep it that way, to remain consistent.
> > Regards
>
> Crypto lz4 accepts u8 * for both compression and decompression:
>
> lz4_compress_crypto(struct crypto_tfm *tfm, const u8 *src,
> unsigned int slen, u8 *dst, unsigned int *dlen)
>
> lz4_decompress_crypto(struct crypto_tfm *tfm, const u8 *src,
> unsigned int slen, u8 *dst, unsigned int *dlen)
>
>
> Internally LZ4 may cast unsigned char* to signed char*, the same way you
> already do with compression:
>
> int lz4_compress(const unsigned char *src, size_t src_len,
> unsigned char *dst, size_t *dst_len, void *wrkmem)
>
> calls:
> lz4_compressctx(void *ctx,
> const char *source, char *dest,
> int isize, int maxoutputsize)
>


+ lib/decompress_unlz4.c
STATIC int INIT decompress(unsigned char *buf, int in_len,
int(*fill)(void*, unsigned int),
int(*flush)(void*, unsigned int),
unsigned char *output,
int *posp,
void(*error)(char *x)

>
> At the moment API is a bit misaligned: unsiged char* for compression and signed char* for
> decompression.
>
>
> My 'real word' use case is, suppose:
>
> struct foo {
> [..]
> int (*compress)(const unsigned char *src, size_t src_len,
> unsigned char *dst, size_t *dst_len, void *wrkmem);
> int (*decompress)(const unsigned char *src, size_t src_len,
> unsigned char *dst, size_t *dst_len);
> };
>
>
> and (for example) module also provides sysfs attribute, so user can switch select
> LZO or LZ4 compressions depending of his needs:
>
> ->compress = lzo1x_1_compress;
> ->decompress = lzo1x_decompress_safe;
>
> to
> ->compress = lz4_compress;
> ->decompress = lz4_decompress_unknownoutputsize;
>
>
> the last one produces unneccessary compilation warning.
>

did you guys have a chance to review the patch? it does not change
implementation/internals, just decompression exported symbols.


-ss

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/