On Sat, Jun 15, 2013 at 03:05:03PM +0200, Daniel Stenberg wrote: > 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? >
Awesome! I've left some minor nitpicks for you inline with the (significantly trimmed) patch. > -- > > / 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. > --- > 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. */ Maybe this comment should be updated to note that fprogress is deprecated? > 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 */ I would have expected this to have been named fxferinfofunc. > 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 ------------------------------------------------------------------- List admin: http://cool.haxx.se/list/listinfo/curl-library Etiquette: http://curl.haxx.se/mail/etiquette.html
