url: alloc the download buffer at transfer start

... and free it as soon as the transfer is done. It removes the extra
alloc when a new size is set with setopt() and reduces memory for unused
easy handles.

In addition: the closure_handle now doesn't use an allocated buffer at
all but the smallest supported size as a stack based one.

Closes #5472

diff --git a/lib/conncache.c b/lib/conncache.c
index 95fcea6f37c5523aa1f390a2abe871c7ccf245c4..4474c28b44062dbddbcf163d01ceb7fec7d1f456 100644 (file)
--- a/lib/conncache.c
+++ b/lib/conncache.c
@@ -531,6 +531,11 @@ Curl_conncache_extract_oldest(struct Curl_easy *data)
 void Curl_conncache_close_all_connections(struct conncache *connc)
 {
   struct connectdata *conn;
+  char buffer[READBUFFER_MIN];
+  if(!connc->closure_handle)
+    return;
+  connc->closure_handle->state.buffer = buffer;
+  connc->closure_handle->set.buffer_size = READBUFFER_MIN;
 
   conn = conncache_find_first_connection(connc);
   while(conn) {
@@ -547,6 +552,7 @@ void Curl_conncache_close_all_connections(struct conncache *connc)
     conn = conncache_find_first_connection(connc);
   }
 
+  connc->closure_handle->state.buffer = NULL;
   if(connc->closure_handle) {
     SIGPIPE_VARIABLE(pipe_st);
     sigpipe_ignore(connc->closure_handle, &pipe_st);

diff --git a/lib/easy.c b/lib/easy.c
index 1deedb26502c0c157e5c4328a542780d6261018d..f58c4521ae0133f9a89048119e5164ad7244a6b2 100644 (file)
--- a/lib/easy.c
+++ b/lib/easy.c
@@ -829,9 +829,6 @@ struct Curl_easy *curl_easy_duphandle(struct Curl_easy *data)
    * the likeliness of us forgetting to init a buffer here in the future.
    */
   outcurl->set.buffer_size = data->set.buffer_size;
-  outcurl->state.buffer = malloc(outcurl->set.buffer_size + 1);
-  if(!outcurl->state.buffer)
-    goto fail;
 
   /* copy all userdefined values */
   if(dupset(outcurl, data))
@@ -947,8 +944,6 @@ struct Curl_easy *curl_easy_duphandle(struct Curl_easy *data)
  */
 void curl_easy_reset(struct Curl_easy *data)
 {
-  long old_buffer_size = data->set.buffer_size;
-
   Curl_free_request_state(data);
 
   /* zero out UserDefined data: */
@@ -972,18 +967,6 @@ void curl_easy_reset(struct Curl_easy *data)
 #if !defined(CURL_DISABLE_HTTP) && !defined(CURL_DISABLE_CRYPTO_AUTH)
   Curl_http_auth_cleanup_digest(data);
 #endif
-
-  /* resize receive buffer */
-  if(old_buffer_size != data->set.buffer_size) {
-    char *newbuff = realloc(data->state.buffer, data->set.buffer_size + 1);
-    if(!newbuff) {
-      DEBUGF(fprintf(stderr, "Error: realloc of buffer failed\n"));
-      /* nothing we can do here except use the old size */
-      data->set.buffer_size = old_buffer_size;
-    }
-    else
-      data->state.buffer = newbuff;
-  }
 }
 
 /*

diff --git a/lib/http2.c b/lib/http2.c
index e4733c94b0595984e06c41e1f798feda050ff85c..7d56e2280f1e696f71fd1703e9287acf1c6f753c 100644 (file)
--- a/lib/http2.c
+++ b/lib/http2.c
@@ -258,15 +258,14 @@ static unsigned int http2_conncheck(struct connectdata *check,
 void Curl_http2_setup_req(struct Curl_easy *data)
 {
   struct HTTP *http = data->req.protop;
-
   http->bodystarted = FALSE;
   http->status_code = -1;
   http->pausedata = NULL;
   http->pauselen = 0;
   http->closed = FALSE;
   http->close_handled = FALSE;
-  http->mem = data->state.buffer;
-  http->len = data->set.buffer_size;
+  http->mem = NULL;
+  http->len = 0;
   http->memlen = 0;
 }
 
@@ -2103,6 +2102,8 @@ CURLcode Curl_http2_setup(struct connectdata *conn)
   struct http_conn *httpc = &conn->proto.httpc;
   struct HTTP *stream = conn->data->req.protop;
 
+  DEBUGASSERT(conn->data->state.buffer);
+
   stream->stream_id = -1;
 
   Curl_dyn_init(&stream->header_recvbuf, DYN_H2_HEADERS);
@@ -2126,6 +2127,8 @@ CURLcode Curl_http2_setup(struct connectdata *conn)
   stream->upload_left = 0;
   stream->upload_mem = NULL;
   stream->upload_len = 0;
+  stream->mem = conn->data->state.buffer;
+  stream->len = conn->data->set.buffer_size;
 
   httpc->inbuflen = 0;
   httpc->nread_inbuf = 0;

diff --git a/lib/multi.c b/lib/multi.c
index 75b0357c3ec5f2b066435ba289b14a8931a4a9f7..b753b1a6146794150ed780633f6c4817786ca278 100644 (file)
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -678,6 +678,7 @@ static CURLcode multi_done(struct Curl_easy *data,
       data->state.lastconnect = NULL;
   }
 
+  Curl_safefree(data->state.buffer);
   Curl_free_request_state(data);
   return result;
 }
@@ -1522,6 +1523,20 @@ static CURLcode protocol_connect(struct connectdata *conn,
   return result; /* pass back status */
 }
 
+/*
+ * preconnect() is called immediately before a connect starts. When a redirect
+ * is followed, this is then called multiple times during a single transfer.
+ */
+static CURLcode preconnect(struct Curl_easy *data)
+{
+  if(!data->state.buffer) {
+    data->state.buffer = malloc(data->set.buffer_size + 1);
+    if(!data->state.buffer)
+      return CURLE_OUT_OF_MEMORY;
+  }
+  return CURLE_OK;
+}
+
 
 static CURLMcode multi_runsingle(struct Curl_multi *multi,
                                  struct curltime now,
@@ -1629,6 +1644,11 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
 
     case CURLM_STATE_CONNECT:
       /* Connect. We want to get a connection identifier filled in. */
+      /* init this transfer. */
+      result = preconnect(data);
+      if(result)
+        break;
+
       Curl_pgrsTime(data, TIMER_STARTSINGLE);
       if(data->set.timeout)
         Curl_expire(data, data->set.timeout, EXPIRE_TIMEOUT);
@@ -2058,7 +2078,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
       char *newurl = NULL;
       bool retry = FALSE;
       bool comeback = FALSE;
-
+      DEBUGASSERT(data->state.buffer);
       /* check if over send speed */
       send_timeout_ms = 0;
       if(data->set.max_send_speed > 0)

diff --git a/lib/setopt.c b/lib/setopt.c
index 72704127c47a7ebfd6de2133d383c7695f4184fd..2ddaeef7d8d49d0fe37bc895b165d66881fafa7d 100644 (file)
--- a/lib/setopt.c
+++ b/lib/setopt.c
@@ -2076,7 +2076,7 @@ CURLcode Curl_vsetopt(struct Curl_easy *data, CURLoption option, va_list param)
       arg = READBUFFER_MIN;
 
     /* Resize if new size */
-    if(arg != data->set.buffer_size) {
+    if((arg != data->set.buffer_size) && data->state.buffer) {
       char *newbuff = realloc(data->state.buffer, arg + 1);
       if(!newbuff) {
         DEBUGF(fprintf(stderr, "Error: realloc of buffer failed\n"));

diff --git a/lib/transfer.c b/lib/transfer.c
index dc43cf6ce14ee765d0eb1f93dd309b96c130114d..ea337ea1daf9fb0d2ebad282f23f6aac56fe0b09 100644 (file)
--- a/lib/transfer.c
+++ b/lib/transfer.c
@@ -558,6 +558,8 @@ static CURLcode readwrite_data(struct Curl_easy *data,
   size_t excess = 0; /* excess bytes read */
   bool readmore = FALSE; /* used by RTP to signal for more data */
   int maxloops = 100;
+  char *buf = data->state.buffer;
+  DEBUGASSERT(buf);
 
   *done = FALSE;
   *comeback = FALSE;
@@ -592,7 +594,7 @@ static CURLcode readwrite_data(struct Curl_easy *data,
 
     if(bytestoread) {
       /* receive data from the network! */
-      result = Curl_read(conn, conn->sockfd, k->buf, bytestoread, &nread);
+      result = Curl_read(conn, conn->sockfd, buf, bytestoread, &nread);
 
       /* read would've blocked */
       if(CURLE_AGAIN == result)
@@ -619,9 +621,8 @@ static CURLcode readwrite_data(struct Curl_easy *data,
     /* indicates data of zero size, i.e. empty file */
     is_empty_data = ((nread == 0) && (k->bodywrites == 0)) ? TRUE : FALSE;
 
-    /* NUL terminate, allowing string ops to be used */
     if(0 < nread || is_empty_data) {
-      k->buf[nread] = 0;
+      buf[nread] = 0;
     }
     else {
       /* if we receive 0 or less here, either the http2 stream is closed or the
@@ -638,7 +639,7 @@ static CURLcode readwrite_data(struct Curl_easy *data,
 
     /* Default buffer to use when we write the buffer, it may be changed
        in the flow below before the actual storing is done. */
-    k->str = k->buf;
+    k->str = buf;
 
     if(conn->handler->readwrite) {
       result = conn->handler->readwrite(data, conn, &nread, &readmore);
@@ -905,10 +906,10 @@ static CURLcode readwrite_data(struct Curl_easy *data,
       /* Parse the excess data */
       k->str += nread;
 
-      if(&k->str[excess] > &k->buf[data->set.buffer_size]) {
+      if(&k->str[excess] > &buf[data->set.buffer_size]) {
         /* the excess amount was too excessive(!), make sure
            it doesn't read out of buffer */
-        excess = &k->buf[data->set.buffer_size] - k->str;
+        excess = &buf[data->set.buffer_size] - k->str;
       }
       nread = (ssize_t)excess;
 
@@ -1467,7 +1468,6 @@ CURLcode Curl_pretransfer(struct Curl_easy *data)
   data->state.authhost.want = data->set.httpauth;
   data->state.authproxy.want = data->set.proxyauth;
   Curl_safefree(data->info.wouldredirect);
-  data->info.wouldredirect = NULL;
 
   if(data->set.httpreq == HTTPREQ_PUT)
     data->state.infilesize = data->set.filesize;

diff --git a/lib/url.c b/lib/url.c
index 5114deaeac12e75612886173f362dd581d3f6251..578844bae4e94d237c35170d1d32745d03b59083 100644 (file)
--- a/lib/url.c
+++ b/lib/url.c
@@ -610,31 +610,21 @@ CURLcode Curl_open(struct Curl_easy **curl)
     return result;
   }
 
-  /* We do some initial setup here, all those fields that can't be just 0 */
-
-  data->state.buffer = malloc(READBUFFER_SIZE + 1);
-  if(!data->state.buffer) {
-    DEBUGF(fprintf(stderr, "Error: malloc of buffer failed\n"));
-    result = CURLE_OUT_OF_MEMORY;
-  }
-  else {
-    result = Curl_init_userdefined(data);
-    if(!result) {
-      Curl_dyn_init(&data->state.headerb, CURL_MAX_HTTP_HEADER);
-      Curl_convert_init(data);
-      Curl_initinfo(data);
+  result = Curl_init_userdefined(data);
+  if(!result) {
+    Curl_dyn_init(&data->state.headerb, CURL_MAX_HTTP_HEADER);
+    Curl_convert_init(data);
+    Curl_initinfo(data);
 
-      /* most recent connection is not yet defined */
-      data->state.lastconnect = NULL;
+    /* most recent connection is not yet defined */
+    data->state.lastconnect = NULL;
 
-      data->progress.flags |= PGRS_HIDE;
-      data->state.current_speed = -1; /* init to negative == impossible */
-    }
+    data->progress.flags |= PGRS_HIDE;
+    data->state.current_speed = -1; /* init to negative == impossible */
   }
 
   if(result) {
     Curl_resolver_cleanup(data->state.resolver);
-    free(data->state.buffer);
     Curl_dyn_free(&data->state.headerb);
     Curl_freeset(data);
     free(data);
@@ -3973,14 +3963,10 @@ CURLcode Curl_init_do(struct Curl_easy *data, struct connectdata *conn)
   k->start = Curl_now(); /* start time */
   k->now = k->start;   /* current time is now */
   k->header = TRUE; /* assume header */
-
   k->bytecount = 0;
-
-  k->buf = data->state.buffer;
   k->ignorebody = FALSE;
 
   Curl_speedinit(data);
-
   Curl_pgrsSetUploadCounter(data, 0);
   Curl_pgrsSetDownloadCounter(data, 0);
 

diff --git a/lib/urldata.h b/lib/urldata.h
index 20ce5ed102595996446c3e64969017b1f910484a..0d756f18e460d082baadd37a70092da4da910745 100644 (file)
--- a/lib/urldata.h
+++ b/lib/urldata.h
@@ -627,7 +627,6 @@ struct SingleRequest {
   struct contenc_writer *writer_stack;
   time_t timeofdoc;
   long bodywrites;
-  char *buf;
   int keepon;
   char *location;   /* This points to an allocated version of the Location:
                        header data */

diff --git a/tests/data/test509 b/tests/data/test509
index 5de1599e1a095dbcd1aefb0a959d354f108ba8d9..0e0dd212b907c6ce7d86c17f19ae0a86328159b8 100644 (file)
--- a/tests/data/test509
+++ b/tests/data/test509
@@ -34,10 +34,7 @@ nothing
 # Verify data after the test has been "shot"
 <verify>
 <stdout>
-seen custom_calloc()
-seen custom_malloc()
-seen custom_realloc()
-seen custom_free()
+Callbacks were invoked!
 </stdout>
 </verify>
 

diff --git a/tests/libtest/lib509.c b/tests/libtest/lib509.c
index e8e803ffcd049176241b339fbdfc3b1a2578200d..1fb2d3445b472b40fe90db193c3fd9f7384362e7 100644 (file)
--- a/tests/libtest/lib509.c
+++ b/tests/libtest/lib509.c
@@ -5,7 +5,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2019, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2020, Daniel Stenberg, <daniel@haxx.se>, et al.
  *
  * This software is licensed as described in the file COPYING, which
  * you should have received as part of this distribution. The terms
@@ -34,70 +34,35 @@
  * memory callbacks which should be calling 'the real thing'.
  */
 
-/*
-#include "memdebug.h"
-*/
-
-static int seen_malloc = 0;
-static int seen_free = 0;
-static int seen_realloc = 0;
-static int seen_strdup = 0;
-static int seen_calloc = 0;
-
-void *custom_malloc(size_t size);
-void custom_free(void *ptr);
-void *custom_realloc(void *ptr, size_t size);
-char *custom_strdup(const char *ptr);
-void *custom_calloc(size_t nmemb, size_t size);
+static int seen;
 
-
-void *custom_calloc(size_t nmemb, size_t size)
+static void *custom_calloc(size_t nmemb, size_t size)
 {
-  if(!seen_calloc) {
-    printf("seen custom_calloc()\n");
-    seen_calloc = 1;
-  }
+  seen++;
   return (calloc)(nmemb, size);
 }
 
-void *custom_malloc(size_t size)
+static void *custom_malloc(size_t size)
 {
-  if(!seen_malloc && seen_calloc) {
-    printf("seen custom_malloc()\n");
-    seen_malloc = 1;
-  }
+  seen++;
   return (malloc)(size);
 }
 
-char *custom_strdup(const char *ptr)
+static char *custom_strdup(const char *ptr)
 {
-  if(!seen_strdup && seen_malloc) {
-    /* currently (2013.03.13), memory tracking enabled builds do not call
-       the strdup callback, in this case malloc callback and memcpy are used
-       instead. If some day this is changed the following printf() should be
-       uncommented, and a line added to test definition.
-    printf("seen custom_strdup()\n");
-    */
-    seen_strdup = 1;
-  }
+  seen++;
   return (strdup)(ptr);
 }
 
-void *custom_realloc(void *ptr, size_t size)
+static void *custom_realloc(void *ptr, size_t size)
 {
-  if(!seen_realloc && seen_malloc) {
-    printf("seen custom_realloc()\n");
-    seen_realloc = 1;
-  }
+  seen++;
   return (realloc)(ptr, size);
 }
 
-void custom_free(void *ptr)
+static void custom_free(void *ptr)
 {
-  if(!seen_free && seen_realloc) {
-    printf("seen custom_free()\n");
-    seen_free = 1;
-  }
+  seen++;
   (free)(ptr);
 }
 
@@ -110,7 +75,6 @@ int test(char *URL)
   CURL *curl;
   int asize;
   char *str = NULL;
-
   (void)URL;
 
   res = curl_global_init_mem(CURL_GLOBAL_ALL,
@@ -136,6 +100,9 @@ int test(char *URL)
   asize = (int)sizeof(a);
   str = curl_easy_escape(curl, (char *)a, asize); /* uses realloc() */
 
+  if(seen)
+    printf("Callbacks were invoked!\n");
+
 test_cleanup:
 
   if(str)