git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: 王健强 <jianqiang.wang@securitygossip.com>
Subject: [PATCH 4/4] progress: use xmalloc/xcalloc
Date: Thu, 11 Apr 2019 09:49:57 -0400	[thread overview]
Message-ID: <20190411134957.GD9182@sigill.intra.peff.net> (raw)
In-Reply-To: <20190411134736.GA28543@sigill.intra.peff.net>

Since the early days of Git, the progress code allocates its struct with
a bare malloc(), not xmalloc(). If the allocation fails, we just avoid
showing progress at all.

While perhaps a noble goal not to fail the whole operation because of
optional progress, in practice:

  1. Any failure to allocate a few dozen bytes here means critical path
     allocations are likely to fail, too.

  2. These days we use a strbuf for throughput progress (and there's a
     patch under discussion to do the same for non-throughput cases,
     too). And that uses xmalloc() under the hood, which means we'd
     still die on some allocation failures.

Let's switch to xmalloc(). That makes us consistent with the rest of Git
and makes it easier to audit for other (less careful) bare mallocs.

Signed-off-by: Jeff King <peff@peff.net>
---
This one is obviously less urgent than the others in that it doesn't
trigger a segfault. So this is purely cleanup.

 progress.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/progress.c b/progress.c
index 5a99c9fbf0..699ac33c4f 100644
--- a/progress.c
+++ b/progress.c
@@ -139,12 +139,10 @@ void display_throughput(struct progress *progress, uint64_t total)
 	now_ns = getnanotime();
 
 	if (!tp) {
-		progress->throughput = tp = calloc(1, sizeof(*tp));
-		if (tp) {
-			tp->prev_total = tp->curr_total = total;
-			tp->prev_ns = now_ns;
-			strbuf_init(&tp->display, 0);
-		}
+		progress->throughput = tp = xcalloc(1, sizeof(*tp));
+		tp->prev_total = tp->curr_total = total;
+		tp->prev_ns = now_ns;
+		strbuf_init(&tp->display, 0);
 		return;
 	}
 	tp->curr_total = total;
@@ -196,13 +194,7 @@ int display_progress(struct progress *progress, uint64_t n)
 static struct progress *start_progress_delay(const char *title, uint64_t total,
 					     unsigned delay)
 {
-	struct progress *progress = malloc(sizeof(*progress));
-	if (!progress) {
-		/* unlikely, but here's a good fallback */
-		fprintf(stderr, "%s...\n", title);
-		fflush(stderr);
-		return NULL;
-	}
+	struct progress *progress = xmalloc(sizeof(*progress));
 	progress->title = title;
 	progress->total = total;
 	progress->last_value = -1;
-- 
2.21.0.922.g1a559e573c

  parent reply	other threads:[~2019-04-11 13:50 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-11 13:47 [PATCH 0/4] use xmalloc in more places Jeff King
2019-04-11 13:48 ` [PATCH 1/4] test-prio-queue: use xmalloc Jeff King
2019-04-11 13:48 ` [PATCH 2/4] xdiff: use git-compat-util Jeff King
2019-04-11 13:49 ` [PATCH 3/4] xdiff: use xmalloc/xrealloc Jeff King
2019-04-11 13:49 ` Jeff King [this message]
2019-04-11 19:14 ` [PATCH 0/4] use xmalloc in more places Taylor Blau
2019-04-11 19:37   ` Jeff King
2019-04-11 19:43     ` Taylor Blau
2019-04-11 20:04       ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190411134957.GD9182@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=jianqiang.wang@securitygossip.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).