From: Martin Ågren <martin.agren@gmail.com>
Date: Mon, 10 Aug 2020 19:47:48 +0000 (+0200)
Subject: progress: don't dereference before checking for NULL
X-Git-Tag: v2.29.0-rc0~159^2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=ac900fddb7fd3cc760ea5c1b79a50ad4564e4a0d;p=thirdparty%2Fgit.git

progress: don't dereference before checking for NULL

In `stop_progress()`, we're careful to check that `p_progress` is
non-NULL before we dereference it, but by then we have already
dereferenced it when calling `finish_if_sparse(*p_progress)`. And, for
what it's worth, we'll go on to blindly dereference it again inside
`stop_progress_msg()`.

We could return early if we get a NULL-pointer, but let's go one step
further and BUG instead. The progress API handles NULL just fine, but
that's the NULL-ness of `*p_progress`, e.g., when running with
`--no-progress`. If `p_progress` is NULL, chances are that's a mistake.
For symmetry, let's do the same check in `stop_progress_msg()`, too.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

diff --git a/progress.c b/progress.c
index 6d2dcff0b6..66ad941cf6 100644
--- a/progress.c
+++ b/progress.c
@@ -319,9 +319,12 @@ static void finish_if_sparse(struct progress *progress)
 
 void stop_progress(struct progress **p_progress)
 {
+	if (!p_progress)
+		BUG("don't provide NULL to stop_progress");
+
 	finish_if_sparse(*p_progress);
 
-	if (p_progress && *p_progress) {
+	if (*p_progress) {
 		trace2_data_intmax("progress", the_repository, "total_objects",
 				   (*p_progress)->total);
 
@@ -342,7 +345,12 @@ void stop_progress(struct progress **p_progress)
 
 void stop_progress_msg(struct progress **p_progress, const char *msg)
 {
-	struct progress *progress = *p_progress;
+	struct progress *progress;
+
+	if (!p_progress)
+		BUG("don't provide NULL to stop_progress_msg");
+
+	progress = *p_progress;
 	if (!progress)
 		return;
 	*p_progress = NULL;