fread_func: move callback pointer from set to state struct

... and assign it from the set.fread_func_set pointer in the
Curl_init_CONNECT function. This A) avoids that we have code that
assigns fields in the 'set' struct (which we always knew was bad) and
more importantly B) it makes it impossibly to accidentally leave the
wrong value for when the handle is re-used etc.

Introducing a state-init functionality in multi.c, so that we can set a
specific function to get called when we enter a state. The
Curl_init_CONNECT is thus called when switching to the CONNECT state.

Bug: https://github.com/bagder/curl/issues/346

lib/ftp.c

@@ -1673,8 +1673,8 @@ static CURLcode ftp_state_ul_setup(struct connectdata *conn,
             BUFSIZE : curlx_sotouz(data->state.resume_from - passed);
 
           size_t actuallyread =
-            data->set.fread_func(data->state.buffer, 1, readthisamountnow,
-                                 data->set.in);
+            data->state.fread_func(data->state.buffer, 1, readthisamountnow,
+                                   data->state.in);
 
           passed += actuallyread;
           if((actuallyread == 0) || (actuallyread > readthisamountnow)) {

lib/http.c

@@ -1001,8 +1001,8 @@ static size_t readmoredata(char *buffer,
       /* move backup data into focus and continue on that */
       http->postdata = http->backup.postdata;
       http->postsize = http->backup.postsize;
-      conn->data->set.fread_func = http->backup.fread_func;
-      conn->data->set.in = http->backup.fread_in;
+      conn->data->state.fread_func = http->backup.fread_func;
+      conn->data->state.in = http->backup.fread_in;
 
       http->sending++; /* move one step up */
 
@@ -1157,14 +1157,14 @@ CURLcode Curl_add_buffer_send(Curl_send_buffer *in,
         ptr = in->buffer + amount;
 
         /* backup the currently set pointers */
-        http->backup.fread_func = conn->data->set.fread_func;
-        http->backup.fread_in = conn->data->set.in;
+        http->backup.fread_func = conn->data->state.fread_func;
+        http->backup.fread_in = conn->data->state.in;
         http->backup.postdata = http->postdata;
         http->backup.postsize = http->postsize;
 
         /* set the new pointers for the request-sending */
-        conn->data->set.fread_func = (curl_read_callback)readmoredata;
-        conn->data->set.in = (void *)conn;
+        conn->data->state.fread_func = (curl_read_callback)readmoredata;
+        conn->data->state.in = (void *)conn;
         http->postdata = ptr;
         http->postsize = (curl_off_t)size;
 
@@ -2162,8 +2162,8 @@ CURLcode Curl_http(struct connectdata *conn, bool *done)
               BUFSIZE : curlx_sotouz(data->state.resume_from - passed);
 
             size_t actuallyread =
-              data->set.fread_func(data->state.buffer, 1, readthisamountnow,
-                                   data->set.in);
+              data->state.fread_func(data->state.buffer, 1, readthisamountnow,
+                                     data->state.in);
 
             passed += actuallyread;
             if((actuallyread == 0) || (actuallyread > readthisamountnow)) {
@@ -2437,11 +2437,11 @@ CURLcode Curl_http(struct connectdata *conn, bool *done)
        on. The data->set.fread_func pointer itself will be changed for the
        multipart case to the function that returns a multipart formatted
        stream. */
-    http->form.fread_func = data->set.fread_func;
+    http->form.fread_func = data->state.fread_func;
 
     /* Set the read function to read from the generated form data */
-    data->set.fread_func = (curl_read_callback)Curl_FormReader;
-    data->set.in = &http->form;
+    data->state.fread_func = (curl_read_callback)Curl_FormReader;
+    data->state.in = &http->form;
 
     http->sending = HTTPSEND_BODY;
 
@@ -2659,8 +2659,8 @@ CURLcode Curl_http(struct connectdata *conn, bool *done)
 
         http->sending = HTTPSEND_BODY;
 
-        data->set.fread_func = (curl_read_callback)readmoredata;
-        data->set.in = (void *)conn;
+        data->state.fread_func = (curl_read_callback)readmoredata;
+        data->state.in = (void *)conn;
 
         /* set the upload size to the progress meter */
         Curl_pgrsSetUploadSize(data, http->postsize);

lib/multi.c

@@ -99,6 +99,9 @@ static const char * const statename[]={
 
 static void multi_freetimeout(void *a, void *b);
 
+/* function pointer called once when switching TO a state */
+typedef void (*init_multistate_func)(struct SessionHandle *data);
+
 /* always use this function to change state, to make debugging easier */
 static void mstate(struct SessionHandle *data, CURLMstate state
 #ifdef DEBUGBUILD
@@ -107,6 +110,12 @@ static void mstate(struct SessionHandle *data, CURLMstate state
 )
 {
   CURLMstate oldstate = data->mstate;
+  static const init_multistate_func finit[CURLM_STATE_LAST-1] = {
+    NULL,
+    NULL,
+    Curl_init_CONNECT, /* CONNECT */
+    /* the rest is NULL too */
+  };
 
 #if defined(DEBUGBUILD) && defined(CURL_DISABLE_VERBOSE_STRINGS)
   (void) lineno;
@@ -136,6 +145,10 @@ static void mstate(struct SessionHandle *data, CURLMstate state
   if(state == CURLM_STATE_COMPLETED)
     /* changing to COMPLETED means there's one less easy handle 'alive' */
     data->multi->num_alive--;
+
+  /* if this state has an init-function, run it */
+  if(finit[state])
+    finit[state](data);
 }
 
 #ifndef DEBUGBUILD

lib/ssh.c

@@ -1740,8 +1740,8 @@ static CURLcode ssh_statemach_act(struct connectdata *conn, bool *block)
                 BUFSIZE : curlx_sotouz(data->state.resume_from - passed);
 
               size_t actuallyread =
-                data->set.fread_func(data->state.buffer, 1, readthisamountnow,
-                                     data->set.in);
+                data->state.fread_func(data->state.buffer, 1,
+                                       readthisamountnow, data->state.in);
 
               passed += actuallyread;
               if((actuallyread == 0) || (actuallyread > readthisamountnow)) {

lib/telnet.c

@@ -1423,8 +1423,8 @@ static CURLcode telnet_do(struct connectdata *conn, bool *done)
       for(;;) {
         if(data->set.is_fread_set) {
           /* read from user-supplied method */
-          result = (int)data->set.fread_func(buf, 1, BUFSIZE - 1,
-                                             data->set.in);
+          result = (int)data->state.fread_func(buf, 1, BUFSIZE - 1,
+                                               data->state.in);
           if(result == CURL_READFUNC_ABORT) {
             keepon = FALSE;
             result = CURLE_READ_ERROR;
@@ -1563,13 +1563,13 @@ static CURLcode telnet_do(struct connectdata *conn, bool *done)
   pfd[0].fd = sockfd;
   pfd[0].events = POLLIN;
 
-  if(data->set.fread_func != (curl_read_callback)fread) {
+  if(data->set.is_fread_set) {
     poll_cnt = 1;
     interval_ms = 100; /* poll user-supplied read function */
   }
   else {
     /* really using fread, so infile is a FILE* */
-    pfd[1].fd = fileno((FILE *)data->set.in);
+    pfd[1].fd = fileno((FILE *)data->state.in);
     pfd[1].events = POLLIN;
     poll_cnt = 2;
     interval_ms = 1 * 1000;
@@ -1628,7 +1628,8 @@ static CURLcode telnet_do(struct connectdata *conn, bool *done)
       }
       else {
         /* read from user-supplied method */
-        nread = (int)data->set.fread_func(buf, 1, BUFSIZE - 1, data->set.in);
+        nread = (int)data->state.fread_func(buf, 1, BUFSIZE - 1,
+                                            data->state.in);
         if(nread == CURL_READFUNC_ABORT) {
           keepon = FALSE;
           break;

lib/transfer.c

@@ -115,8 +115,8 @@ CURLcode Curl_fillreadbuffer(struct connectdata *conn, int bytes, int *nreadp)
 
   /* this function returns a size_t, so we typecast to int to prevent warnings
      with picky compilers */
-  nread = (int)data->set.fread_func(data->req.upload_fromhere, 1,
-                                    buffersize, data->set.in);
+  nread = (int)data->state.fread_func(data->req.upload_fromhere, 1,
+                                      buffersize, data->state.in);
 
   if(nread == CURL_READFUNC_ABORT) {
     failf(data, "operation aborted by callback");
@@ -289,8 +289,8 @@ CURLcode Curl_readrewind(struct connectdata *conn)
       /* If no CURLOPT_READFUNCTION is used, we know that we operate on a
          given FILE * stream and we can actually attempt to rewind that
          ourselves with fseek() */
-      if(data->set.fread_func == (curl_read_callback)fread) {
-        if(-1 != fseek(data->set.in, 0, SEEK_SET))
+      if(data->state.fread_func == (curl_read_callback)fread) {
+        if(-1 != fseek(data->state.in, 0, SEEK_SET))
           /* successful rewind */
           return CURLE_OK;
       }
@@ -1286,8 +1286,18 @@ long Curl_sleep_time(curl_off_t rate_bps, curl_off_t cur_rate_bps,
   return (long)rv;
 }
 
+/* Curl_init_CONNECT() gets called each time the handle switches to CONNECT
+   which means this gets called once for each subsequent redirect etc */
+void Curl_init_CONNECT(struct SessionHandle *data)
+{
+  data->state.fread_func = data->set.fread_func_set;
+  data->state.in = data->set.in_set;
+}
+
 /*
- * Curl_pretransfer() is called immediately before a transfer starts.
+ * Curl_pretransfer() is called immediately before a transfer starts, and only
+ * once for one transfer no matter if it has redirects or do multi-pass
+ * authentication etc.
  */
 CURLcode Curl_pretransfer(struct SessionHandle *data)
 {

lib/transfer.h

@@ -22,6 +22,8 @@
  *
  ***************************************************************************/
 
+void Curl_init_CONNECT(struct SessionHandle *data);
+
 CURLcode Curl_pretransfer(struct SessionHandle *data);
 CURLcode Curl_second_connect(struct connectdata *conn);
 CURLcode Curl_posttransfer(struct SessionHandle *data);

lib/url.c

@@ -496,14 +496,14 @@ CURLcode Curl_init_userdefined(struct UserDefined *set)
   CURLcode result = CURLE_OK;
 
   set->out = stdout; /* default output to stdout */
-  set->in  = stdin;  /* default input from stdin */
+  set->in_set = stdin;  /* default input from stdin */
   set->err  = stderr;  /* default stderr to stderr */
 
   /* use fwrite as default function to store output */
   set->fwrite_func = (curl_write_callback)fwrite;
 
   /* use fread as default function to read input */
-  set->fread_func = (curl_read_callback)fread;
+  set->fread_func_set = (curl_read_callback)fread;
   set->is_fread_set = 0;
   set->is_fwrite_set = 0;
 
@@ -1567,7 +1567,7 @@ CURLcode Curl_setopt(struct SessionHandle *data, CURLoption option,
      * FILE pointer to read the file to be uploaded from. Or possibly
      * used as argument to the read callback.
      */
-    data->set.in = va_arg(param, void *);
+    data->set.in_set = va_arg(param, void *);
     break;
   case CURLOPT_INFILESIZE:
     /*
@@ -1862,11 +1862,11 @@ CURLcode Curl_setopt(struct SessionHandle *data, CURLoption option,
     /*
      * Read data callback
      */
-    data->set.fread_func = va_arg(param, curl_read_callback);
-    if(!data->set.fread_func) {
+    data->set.fread_func_set = va_arg(param, curl_read_callback);
+    if(!data->set.fread_func_set) {
       data->set.is_fread_set = 0;
       /* When set to NULL, reset to our internal default function */
-      data->set.fread_func = (curl_read_callback)fread;
+      data->set.fread_func_set = (curl_read_callback)fread;
     }
     else
       data->set.is_fread_set = 1;

lib/urldata.h

@@ -1312,6 +1312,9 @@ struct UrlState {
   bool done; /* set to FALSE when Curl_do() is called and set to TRUE when
                 Curl_done() is called, to prevent Curl_done() to get invoked
                 twice when the multi interface is used. */
+
+  curl_read_callback fread_func; /* read callback/function */
+  void *in;                      /* CURLOPT_READDATA */
 };
 
 
@@ -1428,7 +1431,7 @@ struct UserDefined {
                      proxy string features a ":[port]" that one will override
                      this. */
   void *out;         /* CURLOPT_WRITEDATA */
-  void *in;          /* CURLOPT_READDATA */
+  void *in_set;      /* CURLOPT_READDATA */
   void *writeheader; /* write the header to this if non-NULL */
   void *rtp_out;     /* write RTP to this if non-NULL */
   long use_port;     /* which port to use (when not using default) */
@@ -1453,7 +1456,7 @@ struct UserDefined {
   curl_write_callback fwrite_func;   /* function that stores the output */
   curl_write_callback fwrite_header; /* function that stores headers */
   curl_write_callback fwrite_rtp;    /* function that stores interleaved RTP */
-  curl_read_callback fread_func;     /* function that reads the input */
+  curl_read_callback fread_func_set; /* function that reads the input */
   int is_fread_set; /* boolean, has read callback been set to non-NULL? */
   int is_fwrite_set; /* boolean, has write callback been set to non-NULL? */
   curl_progress_callback fprogress; /* OLD and deprecated progress callback  */