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 <daniel_at_haxx.se>
> 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, <daniel_at_haxx.se>, et al.
> + * Copyright (C) 1998 - 2013, Daniel Stenberg, <daniel_at_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
> @@ -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
Received on 2013-06-17