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

memory pointer ownership #41

Open
smdjeff opened this issue Nov 10, 2021 · 2 comments
Open

memory pointer ownership #41

smdjeff opened this issue Nov 10, 2021 · 2 comments
Labels

Comments

@smdjeff
Copy link

smdjeff commented Nov 10, 2021

it wasn't clear from the documentation... but uzlib_compress() allocates memory for out.outbuf
uzlib_compress() -> literal() -> zlib_literal() -> outbits() -> sresize()

it's odd that the comp.hash_table is allocated for uzlib by the app, but that uzlib allocates comp.out.outbuf on its own. it'd be cleaner if it was one or the other party doing the allocs.

I'd suggest this be documented (via the author's self documenting style) with a simple change to example/tgzip.c

comp.out.outbuf = malloc(0);
...
uzlib_compress(&comp, source, len);
...
free(comp.out.outbuf);
@pfalcon
Copy link
Owner

pfalcon commented Nov 11, 2021

The short reply that uzlib follows dependency injection pattern re: memory allocation. The longer reply is that while what the short reply says is true, but some parts are taken from other projects, like defl_static.c, which have some "stray" code. The idea so far was that you should use uzlib compression in a way to not hit that stray code. Granted, it's a bit confusing. Actually, the compression support in uzlib was quickly put up as a kind of proof of concept, and needs various improvements and refactoring, as mentioned in the corresponding README section: https://github.com/pfalcon/uzlib#compressor-features.

So far, I didn't have a chance to get to those (like, no project/customer for compression, as most projects are interested in decompression). I'll try to queue up at least some refactorings for the coming months.

@mguetschow
Copy link

The memory allocated during compression in the line mentioned in the initial post is actually also never freed, eventually filling up the memory after a certain number of compressions.

I think a clear warning should be added to the README to not use the compression part in production code until this is resolved.

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

No branches or pull requests

3 participants