lib: sometimes we call Curl_dyn_free() on uninitialized dynbufs

[bagder]

I did this

I added an assert in Curl_dyn_free() to make sure we only free dynbufs that are properly initialized first. (See patch below.)

This leads to a number of failures where existing code assumes that freeing uninitialized dynbufs is fine. As long as the struct is otherwise cleared, this is actually fine, but is a fragile thing to depend on. Plus, without the assert we cannot easily detect if the free is done on a dynbuf that was not cleared.

--- a/lib/dynbuf.c
+++ b/lib/dynbuf.c
@@ -57,10 +57,11 @@ void Curl_dyn_init(struct dynbuf *s, size_t toobig)
  * 'init' field and thus this buffer can be reused to add data to again.
  */
 void Curl_dyn_free(struct dynbuf *s)
 {
   DEBUGASSERT(s);
+  DEBUGASSERT(s->init == DYNINIT);
   Curl_safefree(s->bufr);
   s->leng = s->allc = 0;
 }
 
 /*

I expected the following

Curl_dyn_free() should only free dynbufs that have been initialized with Curl_dyn_init()-

curl/libcurl version

git master

operating system

all

[jay]

Is there a reason this can't be solved by adding Curl_dyn_init calls where they're missing? Do you think of this as a problem of missing Curl_dyn_init calls or a problem of Curl_dyn_free being called indiscriminately?

[bagder]

I believe most of them are cases where we do the cleanup procedure that includes the calls to Curl_dyn_free independently of what exactly was inited. The fixes for these are then probably to either make sure we do the init (earlier), or we make sure to skip the free if no init was ever done. I presume there could be a little of both.

[icing]

made #16775 to add the assert and fix the failures.