Skip to content

Commit

Permalink
fread_func: move callback pointer from set to state struct
Browse files Browse the repository at this point in the history
... 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: #346
  • Loading branch information
bagder committed Oct 15, 2015
1 parent 854976a commit d363db6
Show file tree
Hide file tree
Showing 9 changed files with 64 additions and 35 deletions.
4 changes: 2 additions & 2 deletions lib/ftp.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
26 changes: 13 additions & 13 deletions lib/http.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */

Expand Down Expand 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;

Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand Down
13 changes: 13 additions & 0 deletions lib/multi.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions lib/ssh.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
11 changes: 6 additions & 5 deletions lib/telnet.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
20 changes: 15 additions & 5 deletions lib/transfer.c
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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)
{
Expand Down
2 changes: 2 additions & 0 deletions lib/transfer.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
12 changes: 6 additions & 6 deletions lib/url.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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:
/*
Expand Down Expand Up @@ -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;
Expand Down
7 changes: 5 additions & 2 deletions lib/urldata.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
};


Expand Down Expand Up @@ -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) */
Expand All @@ -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 */
Expand Down

0 comments on commit d363db6

Please sign in to comment.