Hi friends,It has been discussed so many times I decided to just make it. Attached here is my first take on a new progress callback that doesn't use any double (floating point) variable types. They were always a bad design choice that have just been following along since the age of dawn.
We're in a feature freeze for another week so this is shown here to attract your feedback and comments. What do you miss in the current callback and is there any other design mistakes with the former one that we can fix while at it?
For example, we can perhaps fix that annoying thing that we set unknown values to 0 instead of signalling it a really nice way... (as 0 can also mean that it is actually known to be 0!)
Thoughts? -- / daniel.haxx.se
From d81177d8ad811bf58e195856e35d25200c2cebfc Mon Sep 17 00:00:00 2001 From: Daniel Stenberg <[email protected]> Date: Sat, 15 Jun 2013 14:57:01 +0200 Subject: [PATCH] CURLOPT_XFERINFOFUNCTION: introducing a new progress callback CURLOPT_XFERINFOFUNCTION is now the preferred progress callback function and CURLOPT_PROGRESSFUNCTION is considered deprecated. This new callback uses pure 'curl_off_t' arguments to pass on full resolution sizes. It otherwise retains the same characteristics: the same call rate, the same meanings for the arguments and the return code is used the same way. --- docs/TODO | 14 ++------------ docs/libcurl/curl_easy_setopt.3 | 39 ++++++++++++++++++++++++++++++++++++++ docs/libcurl/symbols-in-versions | 4 +++- include/curl/curl.h | 22 +++++++++++++++++++-- lib/progress.c | 17 ++++++++++++++--- lib/url.c | 12 ++++++++++++ lib/urldata.h | 3 ++- 7 files changed, 92 insertions(+), 19 deletions(-) diff --git a/docs/TODO b/docs/TODO index 127e2b8..88b3efb 100644 --- a/docs/TODO +++ b/docs/TODO @@ -16,8 +16,7 @@ 1.3 struct lifreq 1.4 signal-based resolver timeouts 1.5 get rid of PATH_MAX - 1.6 progress callback without doubles - 1.7 Happy Eyeball dual stack connect + 1.6 Happy Eyeball dual stack connect 2. libcurl - multi interface 2.1 More non-blocking @@ -157,16 +156,7 @@ we need libssh2 to properly tell us when we pass in a too small buffer and its current API (as of libssh2 1.2.7) doesn't. -1.6 progress callback without doubles - - The progress callback was introduced way back in the days and the choice to - use doubles in the arguments was possibly good at the time. Today the doubles - only confuse users and make the amounts less precise. We should introduce - another progress callback option that take precedence over the old one and - have both co-exist for a forseeable time until we can remove the double-using - one. - -1.7 Happy Eyeball dual stack connect +1.6 Happy Eyeball dual stack connect In order to make alternative technologies not suffer when transitioning, like when introducing IPv6 as an alternative to IPv4 and there are more than one diff --git a/docs/libcurl/curl_easy_setopt.3 b/docs/libcurl/curl_easy_setopt.3 index e5ca09f..60bdde4 100644 --- a/docs/libcurl/curl_easy_setopt.3 +++ b/docs/libcurl/curl_easy_setopt.3 @@ -377,6 +377,45 @@ function that performs transfers. \fICURLOPT_NOPROGRESS\fP must be set to 0 to make this function actually get called. +.IP CURLOPT_XFERINFOFUNCTION +Pass a pointer to a function that matches the following prototype: + +.nf +\fBint function(void *clientp, curl_off_t dltotal, curl_off_t dlnow, + curl_off_t ultotal, curl_off_t ulnow);\fP +.fi + +This function gets called by libcurl instead of its internal equivalent with a +frequent interval. While data is being transferred it will be called very +frequently, and during slow periods like when nothing is being transferred it +can slow down to about one call per second. + +\fIclientp\fP is the pointer set with \fICURLOPT_XFERINFODATA\fP, it is only +passed along from the application to the callback. + +The callback gets told how much data libcurl will transfer and has +transferred, in number of bytes. \fIdltotal\fP is the total number of bytes +libcurl expects to download in this transfer. \fIdlnow\fP is the number of +bytes downloaded so far. \fIultotal\fP is the total number of bytes libcurl +expects to upload in this transfer. \fIulnow\fP is the number of bytes +uploaded so far. + +Unknown/unused argument values passed to the callback will be set to zero +(like if you only download data, the upload size will remain 0). Many times +the callback will be called one or more times first, before it knows the data +sizes so a program must be made to handle that. + +Returning a non-zero value from this callback will cause libcurl to abort the +transfer and return \fICURLE_ABORTED_BY_CALLBACK\fP. + +If you transfer data with the multi interface, this function will not be +called during periods of idleness unless you call the appropriate libcurl +function that performs transfers. + +\fICURLOPT_NOPROGRESS\fP must be set to 0 to make this function actually +get called. + +(Added in 7.32.0) .IP CURLOPT_PROGRESSDATA Pass a pointer that will be untouched by libcurl and passed as the first argument in the progress callback set with \fICURLOPT_PROGRESSFUNCTION\fP. diff --git a/docs/libcurl/symbols-in-versions b/docs/libcurl/symbols-in-versions index 54b6dbc..e61cbbe 100644 --- a/docs/libcurl/symbols-in-versions +++ b/docs/libcurl/symbols-in-versions @@ -428,7 +428,7 @@ CURLOPT_POSTREDIR 7.19.1 CURLOPT_PREQUOTE 7.9.5 CURLOPT_PRIVATE 7.10.3 CURLOPT_PROGRESSDATA 7.1 -CURLOPT_PROGRESSFUNCTION 7.1 +CURLOPT_PROGRESSFUNCTION 7.1 7.32.0 CURLOPT_PROTOCOLS 7.19.4 CURLOPT_PROXY 7.1 CURLOPT_PROXYAUTH 7.10.7 @@ -525,6 +525,8 @@ CURLOPT_WRITEDATA 7.9.7 CURLOPT_WRITEFUNCTION 7.1 CURLOPT_WRITEHEADER 7.1 CURLOPT_WRITEINFO 7.1 +CURLOPT_XFERINFODATA 7.32.0 +CURLOPT_XFERINFOFUNCTION 7.32.0 CURLPAUSE_ALL 7.18.0 CURLPAUSE_CONT 7.18.0 CURLPAUSE_RECV 7.18.0 diff --git a/include/curl/curl.h b/include/curl/curl.h index e8ec9ee..41c0881 100644 --- a/include/curl/curl.h +++ b/include/curl/curl.h @@ -156,12 +156,22 @@ struct curl_httppost { HTTPPOST_CALLBACK posts */ }; +/* This is the CURLOPT_PROGRESSFUNCTION callback proto. It is now considered + deprecated but was the only choice up until 7.31.0 */ typedef int (*curl_progress_callback)(void *clientp, double dltotal, double dlnow, double ultotal, double ulnow); +/* This is the CURLOPT_XFERINFOFUNCTION callback proto. It was introduced in + 7.32.0, it avoids floating point and provides more detailed information. */ +typedef int (*curl_xferinfo_callback)(void *clientp, + curl_off_t dltotal, + curl_off_t dlnow, + curl_off_t ultotal, + curl_off_t ulnow); + #ifndef CURL_MAX_WRITE_SIZE /* Tests have proven that 20K is a very bad buffer size for uploads on Windows, while 16K for some odd reason performed a lot better. @@ -968,13 +978,16 @@ typedef enum { /* 55 = OBSOLETE */ - /* Function that will be called instead of the internal progress display + /* DEPRECATED + * Function that will be called instead of the internal progress display * function. This function should be defined as the curl_progress_callback * prototype defines. */ CINIT(PROGRESSFUNCTION, FUNCTIONPOINT, 56), - /* Data passed to the progress callback */ + /* Data passed to the CURLOPT_PROGRESSFUNCTION and CURLOPT_XFERINFOFUNCTION + callbacks */ CINIT(PROGRESSDATA, OBJECTPOINT, 57), +#define CURLOPT_XFERINFODATA CURLOPT_PROGRESSDATA /* We want the referrer field set automatically when following locations */ CINIT(AUTOREFERER, LONG, 58), @@ -1533,6 +1546,11 @@ typedef enum { /* Enable/disable SASL initial response */ CINIT(SASL_IR, LONG, 218), + /* Function that will be called instead of the internal progress display + * function. This function should be defined as the curl_xferinfo_callback + * prototype defines. (Deprecates CURLOPT_PROGRESSFUNCTION) */ + CINIT(XFERINFOFUNCTION, FUNCTIONPOINT, 219), + CURLOPT_LASTENTRY /* the last unused */ } CURLoption; diff --git a/lib/progress.c b/lib/progress.c index 4cff2b7..b66c14d 100644 --- a/lib/progress.c +++ b/lib/progress.c @@ -5,7 +5,7 @@ * | (__| |_| | _ <| |___ * \___|\___/|_| \_\_____| * - * Copyright (C) 1998 - 2012, Daniel Stenberg, <[email protected]>, et al. + * Copyright (C) 1998 - 2013, Daniel Stenberg, <[email protected]>, et al. * * This software is licensed as described in the file COPYING, which * you should have received as part of this distribution. The terms @@ -357,10 +357,21 @@ int Curl_pgrsUpdate(struct connectdata *conn) } /* Calculations end */ if(!(data->progress.flags & PGRS_HIDE)) { - /* progress meter has not been shut off */ - if(data->set.fprogress) { + if(data->set.fprogressfunc) { + /* There's a callback set, so we call that instead of writing + anything ourselves. This really is the way to go. */ + result= data->set.fprogressfunc(data->set.progress_client, + data->progress.size_dl, + data->progress.downloaded, + data->progress.size_ul, + data->progress.uploaded); + if(result) + failf(data, "Callback aborted"); + return result; + } + else if(data->set.fprogress) { /* There's a callback set, so we call that instead of writing anything ourselves. This really is the way to go. */ result= data->set.fprogress(data->set.progress_client, diff --git a/lib/url.c b/lib/url.c index e116a5b..2413558 100644 --- a/lib/url.c +++ b/lib/url.c @@ -1598,8 +1598,20 @@ CURLcode Curl_setopt(struct SessionHandle *data, CURLoption option, data->progress.callback = TRUE; /* no longer internal */ else data->progress.callback = FALSE; /* NULL enforces internal */ + break; + + case CURLOPT_XFERINFOFUNCTION: + /* + * Transfer info callback function + */ + data->set.fprogressfunc = va_arg(param, curl_xferinfo_callback); + if(data->set.fprogressfunc) + data->progress.callback = TRUE; /* no longer internal */ + else + data->progress.callback = FALSE; /* NULL enforces internal */ break; + case CURLOPT_PROGRESSDATA: /* * Custom client data to pass to the progress callback diff --git a/lib/urldata.h b/lib/urldata.h index 0c07cab..e8d6125 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -1419,7 +1419,8 @@ struct UserDefined { curl_read_callback fread_func; /* 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; /* function for progress information */ + curl_progress_callback fprogress; /* OLD progress callback */ + curl_xferinfo_callback fprogressfunc; /* progress callback */ curl_debug_callback fdebug; /* function that write informational data */ curl_ioctl_callback ioctl_func; /* function for I/O control */ curl_sockopt_callback fsockopt; /* function for setting socket options */ -- 1.7.10.4
------------------------------------------------------------------- List admin: http://cool.haxx.se/list/listinfo/curl-library Etiquette: http://curl.haxx.se/mail/etiquette.html
