Hi Ken,
According to the docs [1]: If deflate returns Z_OK and with zero
avail_out, it must be called again after making room in the buffer
because there might be more output pending.
On the other hand it says: Z_FINISH can be used in the first deflate
call after deflateInit if all the compression is to be done in a single
step. In order to complete in one call, avail_out must be at least the
value returned by deflateBound. Then deflate is guaranteed to return
Z_STREAM_END.
But: Note that it is possible for the compressed size to be larger than
the value returned by deflateBound() if flush options other than
Z_FINISH or Z_NO_FLUSH are used.
At the beginning of zlib_compress() there's code that decides with which
flush option to call deflate().
/* Only flush for static content or on last (zero-length) chunk */
If it's possible to make flush to be always Z_FINISH, then I guess the
entire
do { ... } while (!zstrm->avail_out);
loop could be simplified to just:
zstrm->next_out = (Bytef *) txn->zbuf.s + txn->zbuf.len;
zstrm->avail_out = txn->zbuf.alloc - txn->zbuf.len;
zr = deflate(zstrm, flush);
if (zr != Z_STREAM_END) {
/* something went wrong */
syslog(LOG_ERR, "zlib deflate error: %d %s", zr, zstrm->msg);
return -1;
}
I changed the "if (zr)" condition as the only good value would be
Z_STREAM_END.
But if the flush option can't be always Z_FINISH, then I believe the
loop should be kept with the checks for !zstrm->avail_out as it's
possible that there would be not enough buffer for deflate() to complete
in one call.
Regards,
Anatoli
[1] https://www.zlib.net/manual.html
On 2/6/20 17:36, Ken Murchison wrote:
> Hi Anatoli,
>
> Thanks for the report. I'm not sure that we even need the
> deflatePending() call, since we use deflateBound() to create an
> appropriately-sized buffer to hold the entire compressed response body.
> Let me do some testing.
>
>
> On 6/2/20 3:48 AM, Anatoli wrote:
>> Cyrus developers,
>>
>> Is it possible to somehow rework the code in imap/httpd.c lines 158-169
>> in order to NOT use deflatePending as this func is not available on
>> OpenBSD? The zlib version there is 1.2.3 and deflatePending appeared in
>> 1.2.5 so the code doesn't compile with --enable-http (undefined symbol:
>> deflatePending). The packaged version disables http for now.
>>
>> Is it safe to reduce these lines:
>>
>> 158 if (!zstrm->avail_out) {
>> 159 unsigned pending;
>> 160
>> 161 zr = deflatePending(zstrm, &pending, Z_NULL);
>> 162 if (zr != Z_OK) {
>> 163 /* something went wrong */
>> 164 syslog(LOG_ERR, "zlib deflate error: %d %s", zr,
>> zstrm->msg);
>> 165 return -1;
>> 166 }
>> 167
>> 168 buf_ensure(&txn->zbuf, pending);
>> 169 }
>>
>>
>> to something like:
>>
>> 158 if (!zstrm->avail_out) {
>> 159 buf_ensure(&txn->zbuf, 256 * 1024);
>> 160 }
>>
>> If I understand it correctly, deflatePending in this case is only used
>> to get the needed buffer size and we could replace it with a hardcoded
>> buffer size (like in my example of 256K).
>>
>>
>> Thanks,
>> Anatoli
>
