[v2,1/4] progress: make display_progress() return void

Commit Message

SZEDER Gábor April 1, 2019, 11:52 a.m. UTC

Ever since the progress infrastructure was introduced in 96a02f8f6d
(common progress display support, 2007-04-18), display_progress() has
returned an int, telling callers whether it updated the progress bar
or not.  However, this is:

  - useless, because over the last dozen years there has never been a
    single caller that cared about that return value.

  - not quite true, because it doesn't print a progress bar when
    running in the background, yet it returns 1; see 85cb8906f0
    (progress: no progress in background, 2015-04-13).

The related display_throughput() function returned void already upon
its introduction in cf84d51c43 (add throughput to progress display,
2007-10-30).

Let's make display_progress() return void, too.

Signed-off-by: SZEDER Gábor <[e-mail]>
---
 progress.c | 15 ++++++++-------
 progress.h |  2 +-
 2 files changed, 9 insertions(+), 8 deletions(-)

Comments

Eric Sunshine April 2, 2019, 5:42 a.m. UTC | #1

On Mon, Apr 1, 2019 at 7:52 AM SZEDER Gábor <[e-mail]> wrote:
> [...]
> Let's make display_progress() return void, too.
>
> Signed-off-by: SZEDER Gábor <[e-mail]>
> ---
> diff --git a/progress.c b/progress.c
> @@ -78,12 +78,12 @@ static int is_foreground_fd(int fd)
> -static int display(struct progress *progress, uint64_t n, const char *done)
> +static void display(struct progress *progress, uint64_t n, const char *done)
>  {
>         if (progress->delay && (!progress_update || --progress->delay))
> -               return 0;
> +               return;

This 'return' needs to stay, but...

> @@ -100,7 +100,7 @@ static int display(struct progress *progress, uint64_t n, const char *done)
>                         progress_update = 0;
> -                       return 1;
> +                       return;
>                 }
>         } else if (progress_update) {
> @@ -109,10 +109,10 @@ static int display(struct progress *progress, uint64_t n, const char *done)
>                 progress_update = 0;
> -               return 1;
> +               return;
>         }
>
> -       return 0;
> +       return;
>  }

... these three 'returns' can all go away. (In fact, the first two
mysteriously disappear in patch 2/4.)

Patch

diff --git a/progress.c b/progress.c
index 5a99c9fbf0..02a20e7d58 100644
--- a/progress.c
+++ b/progress.c
@@ -78,12 +78,12 @@  static int is_foreground_fd(int fd)
 	return tpgrp < 0 || tpgrp == getpgid(0);
 }
 
-static int display(struct progress *progress, uint64_t n, const char *done)
+static void display(struct progress *progress, uint64_t n, const char *done)
 {
 	const char *eol, *tp;
 
 	if (progress->delay && (!progress_update || --progress->delay))
-		return 0;
+		return;
 
 	progress->last_value = n;
 	tp = (progress->throughput) ? progress->throughput->display.buf : "";
@@ -100,7 +100,7 @@  static int display(struct progress *progress, uint64_t n, const char *done)
 				fflush(stderr);
 			}
 			progress_update = 0;
-			return 1;
+			return;
 		}
 	} else if (progress_update) {
 		if (is_foreground_fd(fileno(stderr)) || done) {
@@ -109,10 +109,10 @@  static int display(struct progress *progress, uint64_t n, const char *done)
 			fflush(stderr);
 		}
 		progress_update = 0;
-		return 1;
+		return;
 	}
 
-	return 0;
+	return;
 }
 
 static void throughput_string(struct strbuf *buf, uint64_t total,
@@ -188,9 +188,10 @@  void display_throughput(struct progress *progress, uint64_t total)
 		display(progress, progress->last_value, NULL);
 }
 
-int display_progress(struct progress *progress, uint64_t n)
+void display_progress(struct progress *progress, uint64_t n)
 {
-	return progress ? display(progress, n, NULL) : 0;
+	if (progress)
+		display(progress, n, NULL);
 }
 
 static struct progress *start_progress_delay(const char *title, uint64_t total,
diff --git a/progress.h b/progress.h
index 70a4d4a0d6..59e40cc4fd 100644
--- a/progress.h
+++ b/progress.h
@@ -4,7 +4,7 @@ 
 struct progress;
 
 void display_throughput(struct progress *progress, uint64_t total);
-int display_progress(struct progress *progress, uint64_t n);
+void display_progress(struct progress *progress, uint64_t n);
 struct progress *start_progress(const char *title, uint64_t total);
 struct progress *start_delayed_progress(const char *title, uint64_t total);
 void stop_progress(struct progress **progress);