madler / zlib

A massively spiffy yet delicately unobtrusive compression library.
http://zlib.net/
Other
5.55k stars 2.43k forks source link

zlib 1.3.1 — Is deflateInit2_ incorrect when -DLIT_MEM? #914

Closed chris0e3 closed 7 months ago

chris0e3 commented 7 months ago

Hello,

Thanks for all your work on zlib. I’ve used it for many years.

I was merging in the latest changes from v1.3.1 to my local copy when I noticed this:

https://github.com/madler/zlib/blob/9f0f2d4f9f1f28be7e16d8bf3b4e9d4ada70aa9f/deflate.c#L496-L497 Should it not be (note very last symbol):

    s->pending_buf = (uchf *) ZALLOC(strm, s->lit_bufsize, LIT_BUFS);
    s->pending_buf_size = (ulg)s->lit_bufsize * LIT_BUFS;

As implied by: https://github.com/madler/zlib/blob/9f0f2d4f9f1f28be7e16d8bf3b4e9d4ada70aa9f/deflate.h#L108


At the same time I also noticed: https://github.com/madler/zlib/blob/9f0f2d4f9f1f28be7e16d8bf3b4e9d4ada70aa9f/deflate.h#L213 should this be written (just for consistency) as:

    int heap[HEAP_SIZE];      /* heap used to build the Huffman trees */

And perhaps this should have the same change (if it’s required to match heap): https://github.com/madler/zlib/blob/9f0f2d4f9f1f28be7e16d8bf3b4e9d4ada70aa9f/deflate.h#L220

Regards,

CHRIS

madler commented 7 months ago

No. In the LIT_MEM case, the actual pending buffer is 80% of the space allocated for the thing called pending_buf. Perhaps a little confusing, but quite intentional. That extra space is needed to keep the pending buffer data from catching up with the symbol data written into the same allocated memory.

Sure, I suppose it could be heap[HEAP_SIZE]. Though I kind of like it the way it is, as it is immediately obvious to someone familiar with building binary trees that you need 2n+1 nodes for n leaves. Maybe I'll get rid of the HEAP_SIZE macro.