xiaoyin0208 / lz4

Automatically exported from code.google.com/p/lz4
0 stars 0 forks source link

No API to finalize work with context #25

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
There are two API calls that work with context - LZ4_compressCtx and 
LZ4_compress64kCtx. But there is not API to free context at the end.

Original issue reported on code.google.com by specialforest on 29 Jun 2012 at 2:44

GoogleCodeExporter commented 8 years ago
Very good point.

I was in fact considering removing these functions, since apparently no one was 
interested in using them.

But if there is some need for them, then, as you suggest, for the sake of 
completeness, functions to free the memory context should be provided too.

Original comment by yann.col...@gmail.com on 1 Jul 2012 at 7:16

GoogleCodeExporter commented 8 years ago
I don't think you should remove them. They aren't widely because there is no 
functions to alloc and free a context. 

You should have LZ4_allocCtx and LZ4_freeCtx. You should also make the 
interface consistent with LZ4HC. LZ4HC already has alloc/free functions. You 
should expose those as well. LZ4_64KLIMIT would also need to be exposed so 
people can use it to determine if they should use LZ4_compress64Ctx or 
LZ4_compressCtx.

Also I recommend converting LZ4_compressBound into a macro and adding one for 
the inverse formula.

#define LZ4_compressBound(isize)   (unsigned)((isize) + ((isize) / 255) + 16)
#define LZ4_compressInbound(osize) (unsigned)(((255 * ((osize) - 16)) / 256) - 
1)

You could even rename them to something that makes more sense like 
LZ4_compressOutputBound(isize) or LZ4_compressInputBound(osize).

Original comment by nathan.m...@gmail.com on 10 Jul 2012 at 9:55

GoogleCodeExporter commented 8 years ago
This definitely makes sense.
I'll look into it.

Original comment by yann.col...@gmail.com on 10 Jul 2012 at 10:53

GoogleCodeExporter commented 8 years ago
This shall be classified as enhancement request.

Original comment by yann.col...@gmail.com on 10 Jul 2012 at 10:53

GoogleCodeExporter commented 8 years ago
On second thought, it might not make any sense to expose Ctx. If the Ctx were 
used to hold the state of the compressor or uncompressor during successive 
calls to LZ4_compress or LZ4_uncompress, then it might make sense to expose it. 
But as it is now, it should probably just remain configurable via a #define in 
the source file as it is now, because it really only affects the compiler/env. 
I'm not sure why you would want to allocate it yourself, when you can simply 
put it on the stack.

Original comment by nathan.m...@gmail.com on 11 Jul 2012 at 3:15

GoogleCodeExporter commented 8 years ago
Functions explicitly handling the context are now removed from the standard *.h 
interface.
It's possible to use them by including directly the *.c, or creating/modifying 
your own *.h

Original comment by yann.col...@gmail.com on 28 Jul 2012 at 1:45