Re: dynbuf: PR #5300

On Fri, 1 May 2020, Patrick Monnerat via curl-library wrote:

> - Implement auto shrink when shortening data under an arbitrary watermark,
> for example 20% of the allocated memory.

I don't think we need that.

The only current code that decreases the buffer sizes do this (they reset the
size to zero) *wanting* to maintain the former alloc size so that it can use
up to the previous buffer length again without having to realloc as an
optimization. That's how we handle reading HTTP headers in both normal HTTP
responses and (starting with my PR) CONNECT responses.

We have no other shrinking use case so implementing support for something else
would be overkill.

If we get a use case where we use dynbuf and want to shrink the allocated
memory, then we can come back and reconsider. But I think it is then better to
do the shrinking explictit rather than automatic.

> - A function to grow allocation without initializing it: something like
> (conceptually) accepting Curl_dyn_addn(&dbuf, NULL, size), maybe returning a
> pointer to the extended memory block.

This was also suggested in the PR (for a future porting over the schannel
logic to dynbuf) but I'm not totally sold on the idea. It complicates the code
substantially, while most of the existing code won't need it. Right now the
buffers are always just content with a size. Very simple, very easy and that's
exactly what all the existing dynbuf users need.

But sure, if we get a good use case or two that show improvements when adding
such a function (in a way that still keeps the simplicity for the rest), then
I'll likely be fine with it.

> - Curl_dyn_head(): opposite of Curl_dyn_tail(), keeping the leading bytes.
> Although very simple to perform without such a function, it'll add
> implementation details abstraction.

Again: use case. I put the *tail function within "#if 0" for now since while
we (in the PR) have talked about a possible need, there is currently none and
since there's no user for it there's no point in providing the function.

An early version of the dynbuf patch had such a "head" function but I removed
it as I didn't see any use for it in the end.

> - In debug mode, store DYNINIT also after the data and check it: this may
> detect buffer overflows. This could also be done before the data, but beware
> of alignement.

With valgrind, fuzzer and sanitizer builds in the CI, I find such "manual"
memory problem detection systems rarely worth it. But sure, I won't be against
merging if someone would implement it.

-- 
  / daniel.haxx.se | Commercial curl support up to 24x7 is available!
                   | Private help, bug fixes, support, ports, new features
                   | https://www.wolfssl.com/contact/
-------------------------------------------------------------------
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:   https://curl.haxx.se/mail/etiquette.html