Re: [PATCH 1/2] progress: create public humanize() to show sizes

> Currently, humanization of downloaded size is done in the same function
> as text formatting. This is an issue if anyone else wants to use this.
>
> Separate text formatting from size simplification and make the function
> public so that it can easily be used by other clients.
>
> We now can use humanize() for both downloaded size and download speed
> calculation. One of the drawbacks is that speed will no look like this
> when download is stalled: "0 bytes/s" instead of "0 KiB/s".
>
> Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
> ---

>  progress.c |   60 ++++++++++++++++++++++++++++++++++--------------------------
>  progress.h |    2 ++
>  2 files changed, 36 insertions(+), 26 deletions(-)
>
> diff --git a/progress.c b/progress.c
> index 3971f49..76c1e42 100644
> --- a/progress.c
> +++ b/progress.c
> @@ -8,8 +8,11 @@
>   * published by the Free Software Foundation.
>   */
>  
> +#include <string.h>
> +

>  #include "git-compat-util.h"
>  #include "progress.h"
> +#include "strbuf.h"
>  
>  #define TP_IDX_MAX      8
>  
> @@ -112,34 +115,33 @@ static int display(struct progress *progress, unsigned n, const char *done)
>  	return 0;
>  }
>  
> -static void throughput_string(struct throughput *tp, off_t total,
> -			      unsigned int rate)
> +void humanize(struct strbuf *buf, off_t bytes)
>  {
> -	int l = sizeof(tp->display);
> -	if (total > 1 << 30) {
> -		l -= snprintf(tp->display, l, ", %u.%2.2u GiB",
> -			      (int)(total >> 30),
> -			      (int)(total & ((1 << 30) - 1)) / 10737419);
> -	} else if (total > 1 << 20) {
> -		int x = total + 5243;  /* for rounding */
> -		l -= snprintf(tp->display, l, ", %u.%2.2u MiB",
> -			      x >> 20, ((x & ((1 << 20) - 1)) * 100) >> 20);
> -	} else if (total > 1 << 10) {
> -		int x = total + 5;  /* for rounding */
> -		l -= snprintf(tp->display, l, ", %u.%2.2u KiB",
> -			      x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);
> +	if (bytes > 1 << 30) {
> +		strbuf_addf(buf, "%u.%2.2u GiB",
> +			    (int)(bytes >> 30),
> +			    (int)(bytes & ((1 << 30) - 1)) / 10737419);
> +	} else if (bytes > 1 << 20) {
> +		int x = bytes + 5243;  /* for rounding */
> +		strbuf_addf(buf, "%u.%2.2u MiB",
> +			    x >> 20, ((x & ((1 << 20) - 1)) * 100) >> 20);
> +	} else if (bytes > 1 << 10) {
> +		int x = bytes + 5;  /* for rounding */
> +		strbuf_addf(buf, "%u.%2.2u KiB",
> +			    x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);
>  	} else {
> -		l -= snprintf(tp->display, l, ", %u bytes", (int)total);
> +		strbuf_addf(buf, "%u bytes", (int)bytes);
>  	}
> +}
>  
> -	if (rate > 1 << 10) {
> -		int x = rate + 5;  /* for rounding */
> -		snprintf(tp->display + sizeof(tp->display) - l, l,
> -			 " | %u.%2.2u MiB/s",
> -			 x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);
> -	} else if (rate)
> -		snprintf(tp->display + sizeof(tp->display) - l, l,
> -			 " | %u KiB/s", rate);
> +static void throughput_string(struct strbuf *buf, off_t total,
> +			      unsigned int rate)
> +{
> +	strbuf_addstr(buf, ", ");
> +	humanize(buf, total);
> +	strbuf_addstr(buf, " | ");
> +	humanize(buf, rate * 1024);
> +	strbuf_addstr(buf, "/s");
>  }
>  
>  void display_throughput(struct progress *progress, off_t total)
> @@ -183,6 +185,7 @@ void display_throughput(struct progress *progress, off_t total)
>  	misecs += (int)(tv.tv_usec - tp->prev_tv.tv_usec) / 977;
>  
>  	if (misecs > 512) {
> +		struct strbuf buf = STRBUF_INIT;
>  		unsigned int count, rate;
>  
>  		count = total - tp->prev_total;
> @@ -197,7 +200,9 @@ void display_throughput(struct progress *progress, off_t total)
>  		tp->last_misecs[tp->idx] = misecs;
>  		tp->idx = (tp->idx + 1) % TP_IDX_MAX;
>  
> -		throughput_string(tp, total, rate);
> +		throughput_string(&buf, total, rate);
> +		strncpy(tp->display, buf.buf, sizeof(tp->display));
> +		strbuf_release(&buf);
>  		if (progress->last_value != -1 && progress_update)
>  			display(progress, progress->last_value, NULL);
>  	}
> @@ -253,9 +258,12 @@ void stop_progress_msg(struct progress **p_progress, const char *msg)
>  
>  		bufp = (len < sizeof(buf)) ? buf : xmalloc(len + 1);
>  		if (tp) {
> +			struct strbuf strbuf = STRBUF_INIT;
>  			unsigned int rate = !tp->avg_misecs ? 0 :
>  					tp->avg_bytes / tp->avg_misecs;
> -			throughput_string(tp, tp->curr_total, rate);
> +			throughput_string(&strbuf, tp->curr_total, rate);
> +			strncpy(tp->display, strbuf.buf, sizeof(tp->display));
> +			strbuf_release(&strbuf);
>  		}
>  		progress_update = 1;
>  		sprintf(bufp, ", %s.\n", msg);
> diff --git a/progress.h b/progress.h
> index 611e4c4..0e70f55 100644
> --- a/progress.h
> +++ b/progress.h
> @@ -2,7 +2,9 @@
>  #define PROGRESS_H
>  
>  struct progress;
> +struct strbuf;
>  
> +void humanize(struct strbuf *buf, off_t bytes);
>  void display_throughput(struct progress *progress, off_t total);
>  int display_progress(struct progress *progress, unsigned n);
>  struct progress *start_progress(const char *title, unsigned total);