pfalcon / uzlib

Radically unbloated DEFLATE/zlib/gzip compression/decompression library. Can decompress any gzip/zlib data, and offers simplified compressor which produces gzip-compatible output, while requiring much less resources (and providing less compression ratio of course).
Other
306 stars 83 forks source link

memory pointer ownership #41

Open smdjeff opened 2 years ago

smdjeff commented 2 years ago

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 commented 2 years ago

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 commented 8 months ago

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.