Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No API to finalize work with context #25

Closed
GoogleCodeExporter opened this issue Aug 12, 2015 · 6 comments
Closed

No API to finalize work with context #25

GoogleCodeExporter opened this issue Aug 12, 2015 · 6 comments

Comments

@GoogleCodeExporter
Copy link

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

This definitely makes sense.
I'll look into it.

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

  • Changed state: Accepted

@GoogleCodeExporter
Copy link
Author

This shall be classified as enhancement request.

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

  • Added labels: Type-Enhancement
  • Removed labels: Type-Defect

@GoogleCodeExporter
Copy link
Author

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
Copy link
Author

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

  • Changed state: Fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant