deflatePending not available in zlib on OpenBSD (undefined symbol)

[Anatoli]

Looks good to me and compiles correctly on OpenBSD.

Could it be included in the next 3.2 release (3.2.2)?


On 2/6/20 19:31, Ken Murchison wrote:
> Yes, you're correct.  We have a comment in prot.c that quotes the same
> passage ion the docs.
> 
> I think this might do the trick:
> 
> 
> diff --git a/imap/httpd.c b/imap/httpd.c
> index fc430d935..b5014b97e 100644
> --- a/imap/httpd.c
> +++ b/imap/httpd.c
> @@ -149,24 +149,12 @@ HIDDEN int zlib_compress(struct transaction_t
> *txn, unsigned flags,
>      zstrm->next_in = (Bytef *) buf;
>      zstrm->avail_in = len;
> 
> -    buf_ensure(&txn->zbuf, deflateBound(zstrm, zstrm->avail_in));
>      buf_reset(&txn->zbuf);
> 
>      do {
>          int zr;
> 
> -        if (!zstrm->avail_out) {
> -            unsigned pending;
> -
> -            zr = deflatePending(zstrm, &pending, Z_NULL);
> -            if (zr != Z_OK) {
> -                /* something went wrong */
> -                syslog(LOG_ERR, "zlib deflate error: %d %s", zr,
> zstrm->msg);
> -                return -1;
> -            }
> -
> -            buf_ensure(&txn->zbuf, pending);
> -        }
> +        buf_ensure(&txn->zbuf, deflateBound(zstrm, len));
> 
>          zstrm->next_out = (Bytef *) txn->zbuf.s + txn->zbuf.len;
>          zstrm->avail_out = txn->zbuf.alloc - txn->zbuf.len;
> 
> 
> On 6/2/20 6:19 PM, Anatoli wrote:
>> 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
>